2021-01-29 07:29:10

by Viresh Kumar

Subject: [PATCH V7 0/6] dt: build overlays

Hi,

This patchset makes necessary changes to the kernel to add support for
building overlays (%.dtbo) and the required fdtoverlay tool. This also
builds static_test.dtb using most of the existing overlay tests present
in drivers/of/unittest-data/ for better test coverage.

Note that in order for anyone to test this stuff, you need to manually
run the ./update-dtc-source.sh script once to fetch the necessary
changes from the external DTC project (i.e. fdtoverlay.c and this[1]
patch).

I have tested this patchset for static and runtime testing (on Hikey
board) and no issues were reported.

V7:
- Add a comment in scripts/dtc/Makefile
- Add Ack from Masahiro for patch 4/6.
- Drop word "merge" from commit log of 2/6.
- Split apply_static_overlay, static_test.dtb, and static_base.dts into
two parts to handle overlay_base.dts and testcases.dts separately.

V6:
- Create separate rules for dtbo-s and separate entries in .gitignore in
4/6 (Masahiro).
- A new file layout for handling all overlays for existing and new tests
5/6 (Frank).
- Include overlay.dts as well now in 6/6 (Frank).

V5:

- Don't reuse DTC_SOURCE for fdtoverlay.c in patch 1/5 (Frank).

- Update .gitignore and scripts/Makefile.dtbinst, drop dtbo-y syntax and
DTC_FLAGS += -@ in patch 4/5 (Masahiro).

- Remove the intermediate dtb, rename output to static_test.dtb, don't
use overlay.dtb and overlay_base.dtb for static builds, improved
layout/comments in Makefile for patch 5/5 (Frank).

--
Viresh

[1] https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc
[2] https://lore.kernel.org/lkml/[email protected]/

Viresh Kumar (6):
scripts: dtc: Fetch fdtoverlay.c from external DTC project
scripts: dtc: Build fdtoverlay tool
scripts: dtc: Remove the unused fdtdump.c file
kbuild: Add support to build overlays (%.dtbo)
of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
of: unittest: Statically apply overlays using fdtoverlay

.gitignore | 1 +
Makefile | 5 +-
drivers/of/unittest-data/Makefile | 56 ++++++
drivers/of/unittest-data/overlay_base.dts | 90 +---------
drivers/of/unittest-data/overlay_common.dtsi | 91 ++++++++++
drivers/of/unittest-data/static_base_1.dts | 4 +
drivers/of/unittest-data/static_base_2.dts | 4 +
drivers/of/unittest-data/testcases.dts | 18 +-
.../of/unittest-data/testcases_common.dtsi | 19 ++
.../of/unittest-data/tests-interrupts.dtsi | 7 -
scripts/Makefile.dtbinst | 3 +
scripts/Makefile.lib | 5 +
scripts/dtc/Makefile | 8 +-
scripts/dtc/fdtdump.c | 163 ------------------
scripts/dtc/update-dtc-source.sh | 3 +-
15 files changed, 204 insertions(+), 273 deletions(-)
create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
create mode 100644 drivers/of/unittest-data/static_base_1.dts
create mode 100644 drivers/of/unittest-data/static_base_2.dts
create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
delete mode 100644 scripts/dtc/fdtdump.c


base-commit: 6ee1d745b7c9fd573fba142a2efdad76a9f1cb04
--
2.25.0.rc1.19.g042ed3e048af

2021-01-29 07:30:09

by Viresh Kumar

Subject: [PATCH V7 2/6] scripts: dtc: Build fdtoverlay tool

We will start building overlays for platforms soon in the kernel and
would need fdtoverlay going forward. Lets start building it.

The fdtoverlay program applies one or more overlay dtb blobs to a base
dtb blob. The kernel build system would later use fdtoverlay to generate
the overlaid blobs based on platform specific configurations.

Signed-off-by: Viresh Kumar <[email protected]>
---
scripts/dtc/Makefile | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 4852bf44e913..c8c21e0f2531 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -1,13 +1,19 @@
# SPDX-License-Identifier: GPL-2.0
# scripts/dtc makefile

-hostprogs-always-$(CONFIG_DTC) += dtc
+hostprogs-always-$(CONFIG_DTC) += dtc fdtoverlay
hostprogs-always-$(CHECK_DT_BINDING) += dtc

dtc-objs := dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
srcpos.o checks.o util.o
dtc-objs += dtc-lexer.lex.o dtc-parser.tab.o

+# The upstream project builds libfdt as a separate library. We are choosing to
+# instead directly link the libfdt object files into fdtoverlay.
+libfdt-objs := fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o
+libfdt = $(addprefix libfdt/,$(libfdt-objs))
+fdtoverlay-objs := $(libfdt) fdtoverlay.o util.o
+
# Source files need to get at the userspace version of libfdt_env.h to compile
HOST_EXTRACFLAGS += -I $(srctree)/$(src)/libfdt

--
2.25.0.rc1.19.g042ed3e048af

2021-01-29 07:30:48

by Viresh Kumar

Subject: [PATCH V7 3/6] scripts: dtc: Remove the unused fdtdump.c file

This was copied from external DTC repository long back and isn't used
anymore. Over that the dtc tool can be used to generate the dts source
back from the dtb. Remove the unused fdtdump.c file.

Signed-off-by: Viresh Kumar <[email protected]>
---
scripts/dtc/fdtdump.c | 163 ------------------------------------------
1 file changed, 163 deletions(-)
delete mode 100644 scripts/dtc/fdtdump.c

diff --git a/scripts/dtc/fdtdump.c b/scripts/dtc/fdtdump.c
deleted file mode 100644
index 7d460a50b513..000000000000
--- a/scripts/dtc/fdtdump.c
+++ /dev/null
@@ -1,163 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * fdtdump.c - Contributed by Pantelis Antoniou <pantelis.antoniou AT gmail.com>
- */
-
-#include <stdint.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <ctype.h>
-
-#include <fdt.h>
-#include <libfdt_env.h>
-
-#include "util.h"
-
-#define ALIGN(x, a) (((x) + ((a) - 1)) & ~((a) - 1))
-#define PALIGN(p, a) ((void *)(ALIGN((unsigned long)(p), (a))))
-#define GET_CELL(p) (p += 4, *((const uint32_t *)(p-4)))
-
-static void print_data(const char *data, int len)
-{
- int i;
- const char *p = data;
-
- /* no data, don't print */
- if (len == 0)
- return;
-
- if (util_is_printable_string(data, len)) {
- printf(" = \"%s\"", (const char *)data);
- } else if ((len % 4) == 0) {
- printf(" = <");
- for (i = 0; i < len; i += 4)
- printf("0x%08x%s", fdt32_to_cpu(GET_CELL(p)),
- i < (len - 4) ? " " : "");
- printf(">");
- } else {
- printf(" = [");
- for (i = 0; i < len; i++)
- printf("%02x%s", *p++, i < len - 1 ? " " : "");
- printf("]");
- }
-}
-
-static void dump_blob(void *blob)
-{
- struct fdt_header *bph = blob;
- uint32_t off_mem_rsvmap = fdt32_to_cpu(bph->off_mem_rsvmap);
- uint32_t off_dt = fdt32_to_cpu(bph->off_dt_struct);
- uint32_t off_str = fdt32_to_cpu(bph->off_dt_strings);
- struct fdt_reserve_entry *p_rsvmap =
- (struct fdt_reserve_entry *)((char *)blob + off_mem_rsvmap);
- const char *p_struct = (const char *)blob + off_dt;
- const char *p_strings = (const char *)blob + off_str;
- uint32_t version = fdt32_to_cpu(bph->version);
- uint32_t totalsize = fdt32_to_cpu(bph->totalsize);
- uint32_t tag;
- const char *p, *s, *t;
- int depth, sz, shift;
- int i;
- uint64_t addr, size;
-
- depth = 0;
- shift = 4;
-
- printf("/dts-v1/;\n");
- printf("// magic:\t\t0x%x\n", fdt32_to_cpu(bph->magic));
- printf("// totalsize:\t\t0x%x (%d)\n", totalsize, totalsize);
- printf("// off_dt_struct:\t0x%x\n", off_dt);
- printf("// off_dt_strings:\t0x%x\n", off_str);
- printf("// off_mem_rsvmap:\t0x%x\n", off_mem_rsvmap);
- printf("// version:\t\t%d\n", version);
- printf("// last_comp_version:\t%d\n",
- fdt32_to_cpu(bph->last_comp_version));
- if (version >= 2)
- printf("// boot_cpuid_phys:\t0x%x\n",
- fdt32_to_cpu(bph->boot_cpuid_phys));
-
- if (version >= 3)
- printf("// size_dt_strings:\t0x%x\n",
- fdt32_to_cpu(bph->size_dt_strings));
- if (version >= 17)
- printf("// size_dt_struct:\t0x%x\n",
- fdt32_to_cpu(bph->size_dt_struct));
- printf("\n");
-
- for (i = 0; ; i++) {
- addr = fdt64_to_cpu(p_rsvmap[i].address);
- size = fdt64_to_cpu(p_rsvmap[i].size);
- if (addr == 0 && size == 0)
- break;
-
- printf("/memreserve/ %llx %llx;\n",
- (unsigned long long)addr, (unsigned long long)size);
- }
-
- p = p_struct;
- while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
-
- /* printf("tag: 0x%08x (%d)\n", tag, p - p_struct); */
-
- if (tag == FDT_BEGIN_NODE) {
- s = p;
- p = PALIGN(p + strlen(s) + 1, 4);
-
- if (*s == '\0')
- s = "/";
-
- printf("%*s%s {\n", depth * shift, "", s);
-
- depth++;
- continue;
- }
-
- if (tag == FDT_END_NODE) {
- depth--;
-
- printf("%*s};\n", depth * shift, "");
- continue;
- }
-
- if (tag == FDT_NOP) {
- printf("%*s// [NOP]\n", depth * shift, "");
- continue;
- }
-
- if (tag != FDT_PROP) {
- fprintf(stderr, "%*s ** Unknown tag 0x%08x\n", depth * shift, "", tag);
- break;
- }
- sz = fdt32_to_cpu(GET_CELL(p));
- s = p_strings + fdt32_to_cpu(GET_CELL(p));
- if (version < 16 && sz >= 8)
- p = PALIGN(p, 8);
- t = p;
-
- p = PALIGN(p + sz, 4);
-
- printf("%*s%s", depth * shift, "", s);
- print_data(t, sz);
- printf(";\n");
- }
-}
-
-
-int main(int argc, char *argv[])
-{
- char *buf;
-
- if (argc < 2) {
- fprintf(stderr, "supply input filename\n");
- return 5;
- }
-
- buf = utilfdt_read(argv[1]);
- if (buf)
- dump_blob(buf);
- else
- return 10;
-
- return 0;
-}
--
2.25.0.rc1.19.g042ed3e048af

2021-01-29 07:30:53

by Viresh Kumar

Subject: [PATCH V7 1/6] scripts: dtc: Fetch fdtoverlay.c from external DTC project

We will start building overlays for platforms soon in the kernel and
would need fdtoverlay tool going forward. Lets start fetching it.

Signed-off-by: Viresh Kumar <[email protected]>
---
scripts/dtc/update-dtc-source.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/dtc/update-dtc-source.sh b/scripts/dtc/update-dtc-source.sh
index bc704e2a6a4a..32ff17ffd089 100755
--- a/scripts/dtc/update-dtc-source.sh
+++ b/scripts/dtc/update-dtc-source.sh
@@ -37,6 +37,7 @@ DTC_SOURCE="checks.c data.c dtc.c dtc.h flattree.c fstree.c livetree.c srcpos.c
LIBFDT_SOURCE="fdt.c fdt.h fdt_addresses.c fdt_empty_tree.c \
fdt_overlay.c fdt_ro.c fdt_rw.c fdt_strerror.c fdt_sw.c \
fdt_wip.c libfdt.h libfdt_env.h libfdt_internal.h"
+FDTOVERLAY_SOURCE=fdtoverlay.c

get_last_dtc_version() {
git log --oneline scripts/dtc/ | grep 'upstream' | head -1 | sed -e 's/^.* \(.*\)/\1/'
@@ -54,7 +55,7 @@ dtc_log=$(git log --oneline ${last_dtc_ver}..)

# Copy the files into the Linux tree
cd $DTC_LINUX_PATH
-for f in $DTC_SOURCE; do
+for f in $DTC_SOURCE $FDTOVERLAY_SOURCE; do
cp ${DTC_UPSTREAM_PATH}/${f} ${f}
git add ${f}
done
--
2.25.0.rc1.19.g042ed3e048af

2021-01-29 07:31:16

by Viresh Kumar

Subject: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

Add support for building DT overlays (%.dtbo). The overlay's source file
will have the usual extension, i.e. .dts, though the blob will have
.dtbo extension to distinguish it from normal blobs.

Acked-by: Masahiro Yamada <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
.gitignore | 1 +
Makefile | 5 ++++-
scripts/Makefile.dtbinst | 3 +++
scripts/Makefile.lib | 5 +++++
4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index d01cda8e1177..bb65fa253e58 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,6 +18,7 @@
*.c.[012]*.*
*.dt.yaml
*.dtb
+*.dtbo
*.dtb.S
*.dwo
*.elf
diff --git a/Makefile b/Makefile
index e0af7a4a5598..d5bc67e523be 100644
--- a/Makefile
+++ b/Makefile
@@ -1337,6 +1337,9 @@ ifneq ($(dtstree),)
%.dtb: include/config/kernel.release scripts_dtc
$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@

+%.dtbo: include/config/kernel.release scripts_dtc
+ $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
+
PHONY += dtbs dtbs_install dtbs_check
dtbs: include/config/kernel.release scripts_dtc
$(Q)$(MAKE) $(build)=$(dtstree)
@@ -1816,7 +1819,7 @@ clean: $(clean-dirs)
@find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \
\( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \
-o -name '*.ko.*' \
- -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
+ -o -name '*.dtb' -o -name '*.dtbo' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
-o -name '*.dwo' -o -name '*.lst' \
-o -name '*.su' -o -name '*.mod' \
-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
diff --git a/scripts/Makefile.dtbinst b/scripts/Makefile.dtbinst
index 50d580d77ae9..ba01f5ba2517 100644
--- a/scripts/Makefile.dtbinst
+++ b/scripts/Makefile.dtbinst
@@ -29,6 +29,9 @@ quiet_cmd_dtb_install = INSTALL $@
$(dst)/%.dtb: $(obj)/%.dtb
$(call cmd,dtb_install)

+$(dst)/%.dtbo: $(obj)/%.dtbo
+ $(call cmd,dtb_install)
+
PHONY += $(subdirs)
$(subdirs):
$(Q)$(MAKE) $(dtbinst)=$@ dst=$(patsubst $(obj)/%,$(dst)/%,$@)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 213677a5ed33..b00855b247e0 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -86,7 +86,9 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(dtb-)

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

# Add subdir path
@@ -327,6 +329,9 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
$(call if_changed_dep,dtc)

+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+ $(call if_changed_dep,dtc)
+
DT_CHECKER ?= dt-validate
DT_BINDING_DIR := Documentation/devicetree/bindings
# DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
--
2.25.0.rc1.19.g042ed3e048af

2021-01-29 07:31:54

by Viresh Kumar

Subject: [PATCH V7 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi

In order to build-test the same unit-test files using fdtoverlay tool,
move the device nodes from the existing overlay_base.dts and
testcases_common.dts files to .dtsi counterparts. The .dts files now
include the new .dtsi files, resulting in exactly the same behavior as
earlier.

The .dtsi files can now be reused for compile time tests using
fdtoverlay (will be done by a later commit).

This is required because the base files passed to fdtoverlay tool
shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
tag).

Note that this commit also moves "testcase-device2" node to
testcases.dts from tests-interrupts.dtsi, as this node has a deliberate
error in it and is only relevant for runtime testing done with
unittest.c.

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/of/unittest-data/overlay_base.dts | 90 +-----------------
drivers/of/unittest-data/overlay_common.dtsi | 91 +++++++++++++++++++
drivers/of/unittest-data/testcases.dts | 18 ++--
.../of/unittest-data/testcases_common.dtsi | 19 ++++
.../of/unittest-data/tests-interrupts.dtsi | 7 --
5 files changed, 118 insertions(+), 107 deletions(-)
create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
create mode 100644 drivers/of/unittest-data/testcases_common.dtsi

diff --git a/drivers/of/unittest-data/overlay_base.dts b/drivers/of/unittest-data/overlay_base.dts
index 99ab9d12d00b..ab9014589c5d 100644
--- a/drivers/of/unittest-data/overlay_base.dts
+++ b/drivers/of/unittest-data/overlay_base.dts
@@ -2,92 +2,4 @@
/dts-v1/;
/plugin/;

-/*
- * Base device tree that overlays will be applied against.
- *
- * Do not add any properties in node "/".
- * Do not add any nodes other than "/testcase-data-2" in node "/".
- * Do not add anything that would result in dtc creating node "/__fixups__".
- * dtc will create nodes "/__symbols__" and "/__local_fixups__".
- */
-
-/ {
- testcase-data-2 {
- #address-cells = <1>;
- #size-cells = <1>;
-
- electric_1: substation@100 {
- compatible = "ot,big-volts-control";
- reg = < 0x00000100 0x100 >;
- status = "disabled";
-
- hvac_1: hvac-medium-1 {
- compatible = "ot,hvac-medium";
- heat-range = < 50 75 >;
- cool-range = < 60 80 >;
- };
-
- spin_ctrl_1: motor-1 {
- compatible = "ot,ferris-wheel-motor";
- spin = "clockwise";
- rpm_avail = < 50 >;
- };
-
- spin_ctrl_2: motor-8 {
- compatible = "ot,roller-coaster-motor";
- };
- };
-
- rides_1: fairway-1 {
- #address-cells = <1>;
- #size-cells = <1>;
- compatible = "ot,rides";
- status = "disabled";
- orientation = < 127 >;
-
- ride@100 {
- #address-cells = <1>;
- #size-cells = <1>;
- compatible = "ot,roller-coaster";
- reg = < 0x00000100 0x100 >;
- hvac-provider = < &hvac_1 >;
- hvac-thermostat = < 29 > ;
- hvac-zones = < 14 >;
- hvac-zone-names = "operator";
- spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
- spin-controller-names = "track_1", "track_2";
- queues = < 2 >;
-
- track@30 {
- reg = < 0x00000030 0x10 >;
- };
-
- track@40 {
- reg = < 0x00000040 0x10 >;
- };
-
- };
- };
-
- lights_1: lights@30000 {
- compatible = "ot,work-lights";
- reg = < 0x00030000 0x1000 >;
- status = "disabled";
- };
-
- lights_2: lights@40000 {
- compatible = "ot,show-lights";
- reg = < 0x00040000 0x1000 >;
- status = "disabled";
- rate = < 13 138 >;
- };
-
- retail_1: vending@50000 {
- reg = < 0x00050000 0x1000 >;
- compatible = "ot,tickets";
- status = "disabled";
- };
-
- };
-};
-
+#include "overlay_common.dtsi"
diff --git a/drivers/of/unittest-data/overlay_common.dtsi b/drivers/of/unittest-data/overlay_common.dtsi
new file mode 100644
index 000000000000..08874a72556e
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_common.dtsi
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Base device tree that overlays will be applied against.
+ *
+ * Do not add any properties in node "/".
+ * Do not add any nodes other than "/testcase-data-2" in node "/".
+ * Do not add anything that would result in dtc creating node "/__fixups__".
+ * dtc will create nodes "/__symbols__" and "/__local_fixups__".
+ */
+
+/ {
+ testcase-data-2 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ electric_1: substation@100 {
+ compatible = "ot,big-volts-control";
+ reg = < 0x00000100 0x100 >;
+ status = "disabled";
+
+ hvac_1: hvac-medium-1 {
+ compatible = "ot,hvac-medium";
+ heat-range = < 50 75 >;
+ cool-range = < 60 80 >;
+ };
+
+ spin_ctrl_1: motor-1 {
+ compatible = "ot,ferris-wheel-motor";
+ spin = "clockwise";
+ rpm_avail = < 50 >;
+ };
+
+ spin_ctrl_2: motor-8 {
+ compatible = "ot,roller-coaster-motor";
+ };
+ };
+
+ rides_1: fairway-1 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "ot,rides";
+ status = "disabled";
+ orientation = < 127 >;
+
+ ride@100 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "ot,roller-coaster";
+ reg = < 0x00000100 0x100 >;
+ hvac-provider = < &hvac_1 >;
+ hvac-thermostat = < 29 > ;
+ hvac-zones = < 14 >;
+ hvac-zone-names = "operator";
+ spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
+ spin-controller-names = "track_1", "track_2";
+ queues = < 2 >;
+
+ track@30 {
+ reg = < 0x00000030 0x10 >;
+ };
+
+ track@40 {
+ reg = < 0x00000040 0x10 >;
+ };
+
+ };
+ };
+
+ lights_1: lights@30000 {
+ compatible = "ot,work-lights";
+ reg = < 0x00030000 0x1000 >;
+ status = "disabled";
+ };
+
+ lights_2: lights@40000 {
+ compatible = "ot,show-lights";
+ reg = < 0x00040000 0x1000 >;
+ status = "disabled";
+ rate = < 13 138 >;
+ };
+
+ retail_1: vending@50000 {
+ reg = < 0x00050000 0x1000 >;
+ compatible = "ot,tickets";
+ status = "disabled";
+ };
+
+ };
+};
+
diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
index a85b5e1c381a..04b9e7bb30d9 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -2,19 +2,15 @@
/dts-v1/;
/plugin/;

+#include "testcases_common.dtsi"
+
/ {
testcase-data {
- changeset {
- prop-update = "hello";
- prop-remove = "world";
- node-remove {
- };
+ testcase-device2 {
+ compatible = "testcase-device";
+ interrupt-parent = <&test_intc2>;
+ interrupts = <1>; /* invalid specifier - too short */
};
};
+
};
-#include "tests-phandle.dtsi"
-#include "tests-interrupts.dtsi"
-#include "tests-match.dtsi"
-#include "tests-address.dtsi"
-#include "tests-platform.dtsi"
-#include "tests-overlay.dtsi"
diff --git a/drivers/of/unittest-data/testcases_common.dtsi b/drivers/of/unittest-data/testcases_common.dtsi
new file mode 100644
index 000000000000..19292bbb4cbb
--- /dev/null
+++ b/drivers/of/unittest-data/testcases_common.dtsi
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/ {
+ testcase-data {
+ changeset {
+ prop-update = "hello";
+ prop-remove = "world";
+ node-remove {
+ };
+ };
+ };
+};
+
+#include "tests-phandle.dtsi"
+#include "tests-interrupts.dtsi"
+#include "tests-match.dtsi"
+#include "tests-address.dtsi"
+#include "tests-platform.dtsi"
+#include "tests-overlay.dtsi"
diff --git a/drivers/of/unittest-data/tests-interrupts.dtsi b/drivers/of/unittest-data/tests-interrupts.dtsi
index ec175e800725..0e5914611107 100644
--- a/drivers/of/unittest-data/tests-interrupts.dtsi
+++ b/drivers/of/unittest-data/tests-interrupts.dtsi
@@ -61,12 +61,5 @@ testcase-device1 {
interrupt-parent = <&test_intc0>;
interrupts = <1>;
};
-
- testcase-device2 {
- compatible = "testcase-device";
- interrupt-parent = <&test_intc2>;
- interrupts = <1>; /* invalid specifier - too short */
- };
};
-
};
--
2.25.0.rc1.19.g042ed3e048af

2021-01-29 07:32:13

by Viresh Kumar

Subject: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

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 <[email protected]>
---
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

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"
--
2.25.0.rc1.19.g042ed3e048af

2021-02-04 03:57:57

by Rob Herring

Subject: Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

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 <[email protected]>
> ---
> 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 <[email protected]>
Date: Wed, 3 Feb 2021 19:22:47 -0600
Subject: [PATCH] rework dtb overlay building

Signed-off-by: Rob Herring <[email protected]>
---
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
--
2.27.0

2021-02-04 07:44:38

by Viresh Kumar

Subject: Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

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.

--
viresh

2021-02-04 16:03:53

by Rob Herring

Subject: Re: [PATCH V7 0/6] dt: build overlays

On Fri, 29 Jan 2021 12:54:04 +0530, Viresh Kumar wrote:
> Hi,
>
> This patchset makes necessary changes to the kernel to add support for
> building overlays (%.dtbo) and the required fdtoverlay tool. This also
> builds static_test.dtb using most of the existing overlay tests present
> in drivers/of/unittest-data/ for better test coverage.
>
> Note that in order for anyone to test this stuff, you need to manually
> run the ./update-dtc-source.sh script once to fetch the necessary
> changes from the external DTC project (i.e. fdtoverlay.c and this[1]
> patch).
>
> I have tested this patchset for static and runtime testing (on Hikey
> board) and no issues were reported.
>
> V7:
> - Add a comment in scripts/dtc/Makefile
> - Add Ack from Masahiro for patch 4/6.
> - Drop word "merge" from commit log of 2/6.
> - Split apply_static_overlay, static_test.dtb, and static_base.dts into
> two parts to handle overlay_base.dts and testcases.dts separately.
>
> V6:
> - Create separate rules for dtbo-s and separate entries in .gitignore in
> 4/6 (Masahiro).
> - A new file layout for handling all overlays for existing and new tests
> 5/6 (Frank).
> - Include overlay.dts as well now in 6/6 (Frank).
>
> V5:
>
> - Don't reuse DTC_SOURCE for fdtoverlay.c in patch 1/5 (Frank).
>
> - Update .gitignore and scripts/Makefile.dtbinst, drop dtbo-y syntax and
> DTC_FLAGS += -@ in patch 4/5 (Masahiro).
>
> - Remove the intermediate dtb, rename output to static_test.dtb, don't
> use overlay.dtb and overlay_base.dtb for static builds, improved
> layout/comments in Makefile for patch 5/5 (Frank).
>
> --
> Viresh
>
> [1] https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc
> [2] https://lore.kernel.org/lkml/[email protected]/
>
> Viresh Kumar (6):
> scripts: dtc: Fetch fdtoverlay.c from external DTC project
> scripts: dtc: Build fdtoverlay tool
> scripts: dtc: Remove the unused fdtdump.c file
> kbuild: Add support to build overlays (%.dtbo)
> of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
> of: unittest: Statically apply overlays using fdtoverlay

I've applied the first 4 patches and did a dtc sync after patch 1.

Rob

2021-02-05 09:10:48

by Geert Uytterhoeven

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

Hi Viresh,

Thanks for your patch
(which I only noticed because it appeared in dt-rh/for-next ;-)

On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> Add support for building DT overlays (%.dtbo). The overlay's source file
> will have the usual extension, i.e. .dts, though the blob will have

Why use .dts and not .dtso for overlays?
Because you originally (until v5) had a single rule for building .dtb
and .dtbo files?

> .dtbo extension to distinguish it from normal blobs.
>
> Acked-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-02-05 09:30:17

by Viresh Kumar

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

On 05-02-21, 10:02, Geert Uytterhoeven wrote:
> Hi Viresh,
>
> Thanks for your patch
> (which I only noticed because it appeared in dt-rh/for-next ;-)
>
> On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> > Add support for building DT overlays (%.dtbo). The overlay's source file
> > will have the usual extension, i.e. .dts, though the blob will have
>
> Why use .dts and not .dtso for overlays?
> Because you originally (until v5) had a single rule for building .dtb
> and .dtbo files?

I am fine with doing that as well if Rob and David agree to it. Rob
did suggest that at one point but we didn't do much about it later on
for some reason.

FWIW, this will also require a change in the DTC compiler.

--
viresh

2021-02-05 09:45:27

by Geert Uytterhoeven

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

Hi Viresh,

On Fri, Feb 5, 2021 at 10:25 AM Viresh Kumar <[email protected]> wrote:
> On 05-02-21, 10:02, Geert Uytterhoeven wrote:
> > Thanks for your patch
> > (which I only noticed because it appeared in dt-rh/for-next ;-)
> >
> > On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> > > Add support for building DT overlays (%.dtbo). The overlay's source file
> > > will have the usual extension, i.e. .dts, though the blob will have
> >
> > Why use .dts and not .dtso for overlays?
> > Because you originally (until v5) had a single rule for building .dtb
> > and .dtbo files?
>
> I am fine with doing that as well if Rob and David agree to it. Rob
> did suggest that at one point but we didn't do much about it later on
> for some reason.
>
> FWIW, this will also require a change in the DTC compiler.

Care to explain why? I've been using .dtsi for ages in
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/renesas-overlays

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-02-05 10:27:48

by Geert Uytterhoeven

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

Hi Viresh,

On Fri, Feb 5, 2021 at 10:55 AM Viresh Kumar <[email protected]> wrote:
> On 05-02-21, 10:41, Geert Uytterhoeven wrote:
> > On Fri, Feb 5, 2021 at 10:25 AM Viresh Kumar <[email protected]> wrote:
> > > On 05-02-21, 10:02, Geert Uytterhoeven wrote:
> > > > Thanks for your patch
> > > > (which I only noticed because it appeared in dt-rh/for-next ;-)
> > > >
> > > > On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> > > > > Add support for building DT overlays (%.dtbo). The overlay's source file
> > > > > will have the usual extension, i.e. .dts, though the blob will have
> > > >
> > > > Why use .dts and not .dtso for overlays?
> > > > Because you originally (until v5) had a single rule for building .dtb
> > > > and .dtbo files?
> > >
> > > I am fine with doing that as well if Rob and David agree to it. Rob
> > > did suggest that at one point but we didn't do much about it later on
> > > for some reason.
> > >
> > > FWIW, this will also require a change in the DTC compiler.
> >
> > Care to explain why? I've been using .dtsi for ages in
> > https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/renesas-overlays
>
> I don't see you building them anywhere, they aren't added to the
> Makefile ever. What am I missing ?
>
> actually none of the dtso's were added to any makefile in that branch.

E.g. "ARM: dts: Build all overlays if OF_OVERLAY=y"?
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=597ee90971687a45678cca8d16bf624d174a99eb

> Anyway, the DTC needs to know how to treat the dtso format and it will
> error out currently with unknown format kind of errors.
>
> Below email [1] have some information on the kind of changes required
> here. Also note that we had to do similar changes for dtbo earlier
> [2].
>
> --
> viresh
>
> [1] https://lore.kernel.org/lkml/CAK7LNASViCOTGR7yDTfh0O+PAu+X-P2NwdY4oPMuXrr51awafA@mail.gmail.com/

-@ is handled by "kbuild: Enable DT symbols when CONFIG_OF_OVERLAY is used"
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=91e9d998514f3743125a707013a30d5f83054579

> [2] https://lore.kernel.org/lkml/30fd0e5f2156665c713cf191c5fea9a5548360c0.1609926856.git.viresh.kumar@linaro.org/

I never had a need for those changes to dtc. .dtso/.dtbo work fine regardless.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-02-05 21:14:00

by Rob Herring

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

On Fri, Feb 05, 2021 at 11:17:10AM +0100, Geert Uytterhoeven wrote:
> Hi Viresh,
>
> On Fri, Feb 5, 2021 at 10:55 AM Viresh Kumar <[email protected]> wrote:
> > On 05-02-21, 10:41, Geert Uytterhoeven wrote:
> > > On Fri, Feb 5, 2021 at 10:25 AM Viresh Kumar <[email protected]> wrote:
> > > > On 05-02-21, 10:02, Geert Uytterhoeven wrote:
> > > > > Thanks for your patch
> > > > > (which I only noticed because it appeared in dt-rh/for-next ;-)
> > > > >
> > > > > On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> > > > > > Add support for building DT overlays (%.dtbo). The overlay's source file
> > > > > > will have the usual extension, i.e. .dts, though the blob will have
> > > > >
> > > > > Why use .dts and not .dtso for overlays?
> > > > > Because you originally (until v5) had a single rule for building .dtb
> > > > > and .dtbo files?
> > > >
> > > > I am fine with doing that as well if Rob and David agree to it. Rob
> > > > did suggest that at one point but we didn't do much about it later on
> > > > for some reason.
> > > >
> > > > FWIW, this will also require a change in the DTC compiler.
> > >
> > > Care to explain why? I've been using .dtsi for ages in
> > > https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/renesas-overlays
> >
> > I don't see you building them anywhere, they aren't added to the
> > Makefile ever. What am I missing ?
> >
> > actually none of the dtso's were added to any makefile in that branch.
>
> E.g. "ARM: dts: Build all overlays if OF_OVERLAY=y"?
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=597ee90971687a45678cca8d16bf624d174a99eb
>
> > Anyway, the DTC needs to know how to treat the dtso format and it will
> > error out currently with unknown format kind of errors.
> >
> > Below email [1] have some information on the kind of changes required
> > here. Also note that we had to do similar changes for dtbo earlier
> > [2].
> >
> > --
> > viresh
> >
> > [1] https://lore.kernel.org/lkml/CAK7LNASViCOTGR7yDTfh0O+PAu+X-P2NwdY4oPMuXrr51awafA@mail.gmail.com/
>
> -@ is handled by "kbuild: Enable DT symbols when CONFIG_OF_OVERLAY is used"
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=91e9d998514f3743125a707013a30d5f83054579
>
> > [2] https://lore.kernel.org/lkml/30fd0e5f2156665c713cf191c5fea9a5548360c0.1609926856.git.viresh.kumar@linaro.org/
>
> I never had a need for those changes to dtc. .dtso/.dtbo work fine regardless.

I think what Viresh means is dtc won't automatically set the input type
to dts if not .dts.

We stuck with .dtbo as it's already widely used. I don't know about
dtso though. If there's strong consensus to use .dtso, then let's do
that. Whatever color for this shed you like.

Rob

2021-02-06 00:59:17

by Viresh Kumar

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

On 05-02-21, 10:41, Geert Uytterhoeven wrote:
> Hi Viresh,
>
> On Fri, Feb 5, 2021 at 10:25 AM Viresh Kumar <[email protected]> wrote:
> > On 05-02-21, 10:02, Geert Uytterhoeven wrote:
> > > Thanks for your patch
> > > (which I only noticed because it appeared in dt-rh/for-next ;-)
> > >
> > > On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> > > > Add support for building DT overlays (%.dtbo). The overlay's source file
> > > > will have the usual extension, i.e. .dts, though the blob will have
> > >
> > > Why use .dts and not .dtso for overlays?
> > > Because you originally (until v5) had a single rule for building .dtb
> > > and .dtbo files?
> >
> > I am fine with doing that as well if Rob and David agree to it. Rob
> > did suggest that at one point but we didn't do much about it later on
> > for some reason.
> >
> > FWIW, this will also require a change in the DTC compiler.
>
> Care to explain why? I've been using .dtsi for ages in
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/renesas-overlays

I don't see you building them anywhere, they aren't added to the
Makefile ever. What am I missing ?

actually none of the dtso's were added to any makefile in that branch.

Anyway, the DTC needs to know how to treat the dtso format and it will
error out currently with unknown format kind of errors.

Below email [1] have some information on the kind of changes required
here. Also note that we had to do similar changes for dtbo earlier
[2].

--
viresh

[1] https://lore.kernel.org/lkml/CAK7LNASViCOTGR7yDTfh0O+PAu+X-P2NwdY4oPMuXrr51awafA@mail.gmail.com/
[2] https://lore.kernel.org/lkml/30fd0e5f2156665c713cf191c5fea9a5548360c0.1609926856.git.viresh.kumar@linaro.org/

2021-02-06 07:55:52

by David Gibson

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

On Fri, Feb 05, 2021 at 02:55:07PM +0530, Viresh Kumar wrote:
> On 05-02-21, 10:02, Geert Uytterhoeven wrote:
> > Hi Viresh,
> >
> > Thanks for your patch
> > (which I only noticed because it appeared in dt-rh/for-next ;-)
> >
> > On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> > > Add support for building DT overlays (%.dtbo). The overlay's source file
> > > will have the usual extension, i.e. .dts, though the blob will have
> >
> > Why use .dts and not .dtso for overlays?
> > Because you originally (until v5) had a single rule for building .dtb
> > and .dtbo files?
>
> I am fine with doing that as well if Rob and David agree to it. Rob
> did suggest that at one point but we didn't do much about it later on
> for some reason.
>
> FWIW, this will also require a change in the DTC compiler.

Not really. It would need a change to automatically recognize that
extension, but you can easily work around that by explicitly giving
the -I option to specify the input type.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachments:

2021-02-06 11:25:16

by Geert Uytterhoeven

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

Hi Rob,

On Fri, Feb 5, 2021 at 10:08 PM Rob Herring <[email protected]> wrote:
> On Fri, Feb 05, 2021 at 11:17:10AM +0100, Geert Uytterhoeven wrote:
> On Fri, Feb 5, 2021 at 10:55 AM Viresh Kumar <[email protected]> wrote:
> > > On 05-02-21, 10:41, Geert Uytterhoeven wrote:
> > > > On Fri, Feb 5, 2021 at 10:25 AM Viresh Kumar <[email protected]> wrote:
> > > > > On 05-02-21, 10:02, Geert Uytterhoeven wrote:
> > > > > > Thanks for your patch
> > > > > > (which I only noticed because it appeared in dt-rh/for-next ;-)
> > > > > >
> > > > > > On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
> > > > > > > Add support for building DT overlays (%.dtbo). The overlay's source file
> > > > > > > will have the usual extension, i.e. .dts, though the blob will have
> > > > > >
> > > > > > Why use .dts and not .dtso for overlays?
> > > > > > Because you originally (until v5) had a single rule for building .dtb
> > > > > > and .dtbo files?
> > > > >
> > > > > I am fine with doing that as well if Rob and David agree to it. Rob
> > > > > did suggest that at one point but we didn't do much about it later on
> > > > > for some reason.
> > > > >
> > > > > FWIW, this will also require a change in the DTC compiler.

> I think what Viresh means is dtc won't automatically set the input type
> to dts if not .dts.

Which is not needed with the kernel build rules.

> We stuck with .dtbo as it's already widely used. I don't know about
> dtso though. If there's strong consensus to use .dtso, then let's do
> that. Whatever color for this shed you like.

I did a quick survey of its popularity, and found:

1. .dts => .dtbo
https://www.kernel.org/doc/Documentation/devicetree/overlay-notes.txt
https://github.com/beagleboard/bb.org-overlays
https://www.raspberrypi.org/documentation/configuration/device-tree.md
https://www.96boards.org/documentation/consumer/dragonboard/dragonboard410c/guides/dt-overlays.md.html
https://learn.adafruit.com/introduction-to-the-beaglebone-black-device-tree/compiling-an-overlay
https://developer.toradex.com/knowledge-base/device-tree-overlays

2. .dtso => .dtbo
https://github.com/linux4sam/dt-overlay-at91/tree/master/sama5d3_xplained
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/renesas-overlays

3. .dtsi => .dtbo
https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18841645/Solution+Zynq+PL+Programming+With+FPGA+Manager

Note that many overlay users haven't migrated to the sugar syntax yet.

IMHO the biggest advantage of .dtso over .dts is that you (and the
tooling) know which files should be compiled to .dtbo, and which to
.dtb. This doesn't matter for

obj-$(CONFIG_FOO) += foo.dtbo

but it does for options like CONFIG_OF_ALL_DTBS.
Having a way to build all overlays will become more important when we
will have more generic overlays (capes/hats/wings/...), that can apply
to multiple base DTBs.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-02-08 11:37:33

by Viresh Kumar

Subject: Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

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 ?

> +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


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.

--
viresh

2021-02-08 14:49:22

by Rob Herring

Subject: Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

On Mon, Feb 8, 2021 at 5:18 AM Viresh Kumar <[email protected]> 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

2021-02-09 04:36:22

by Viresh Kumar

Subject: Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

On 08-02-21, 08:21, Rob Herring wrote:
> In string 'm' replace '.dtb' with '-dtbs'. Then we get the value of
> that variable.

Ah, thanks. I was able to get everything to work as expected now :)

> > 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.

It works with your patch itself, just that it was done after the
failure and so wasn't happening.

> > 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.

I did this instead and it made everything work, we don't try dt.yaml
for the test1.dtb file anymore, is this acceptable ?

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 886d2e6c58f0..b86ff1b3de14 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -66,7 +66,7 @@ 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)),))
+real-dtb-y := $(foreach m,$(overlay-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
dtb-y += $(real-dtb-y)

always-y += $(always-m)

diff --git a/arch/arm64/boot/dts/hisilicon/Makefile b/arch/arm64/boot/dts/hisilicon/Makefile
index f4d68caeba83..69ca27014e89 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
+overlay-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>;
+ };
+};

--
viresh

2021-02-09 10:27:13

by Viresh Kumar

Subject: Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

On 08-02-21, 08:21, Rob Herring wrote:
> We may need to turn off
> checks in overlays for required properties as they may be incomplete.
> We already do that on disabled nodes.

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 :)

Author: Viresh Kumar <[email protected]>
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 <[email protected]>
---
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)

--
viresh

2021-02-16 10:42:14

by Viresh Kumar

Subject: Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

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 <[email protected]>
> 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 <[email protected]>
> ---
> 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)

--
viresh

2021-02-18 21:06:08

by Frank Rowand

Subject: Re: [PATCH V7 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi

On 1/29/21 1:24 AM, Viresh Kumar wrote:
> In order to build-test the same unit-test files using fdtoverlay tool,
> move the device nodes from the existing overlay_base.dts and
> testcases_common.dts files to .dtsi counterparts. The .dts files now
> include the new .dtsi files, resulting in exactly the same behavior as
> earlier.
>
> The .dtsi files can now be reused for compile time tests using
> fdtoverlay (will be done by a later commit).
>
> This is required because the base files passed to fdtoverlay tool
> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
> tag).
>
> Note that this commit also moves "testcase-device2" node to
> testcases.dts from tests-interrupts.dtsi, as this node has a deliberate
> error in it and is only relevant for runtime testing done with
> unittest.c.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/of/unittest-data/overlay_base.dts | 90 +-----------------
> drivers/of/unittest-data/overlay_common.dtsi | 91 +++++++++++++++++++
> drivers/of/unittest-data/testcases.dts | 18 ++--
> .../of/unittest-data/testcases_common.dtsi | 19 ++++
> .../of/unittest-data/tests-interrupts.dtsi | 7 --
> 5 files changed, 118 insertions(+), 107 deletions(-)
> create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
> create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
>
> diff --git a/drivers/of/unittest-data/overlay_base.dts b/drivers/of/unittest-data/overlay_base.dts
> index 99ab9d12d00b..ab9014589c5d 100644
> --- a/drivers/of/unittest-data/overlay_base.dts
> +++ b/drivers/of/unittest-data/overlay_base.dts
> @@ -2,92 +2,4 @@
> /dts-v1/;
> /plugin/;
>
> -/*
> - * Base device tree that overlays will be applied against.
> - *
> - * Do not add any properties in node "/".
> - * Do not add any nodes other than "/testcase-data-2" in node "/".
> - * Do not add anything that would result in dtc creating node "/__fixups__".
> - * dtc will create nodes "/__symbols__" and "/__local_fixups__".
> - */
> -
> -/ {
> - testcase-data-2 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> -
> - electric_1: substation@100 {
> - compatible = "ot,big-volts-control";
> - reg = < 0x00000100 0x100 >;
> - status = "disabled";
> -
> - hvac_1: hvac-medium-1 {
> - compatible = "ot,hvac-medium";
> - heat-range = < 50 75 >;
> - cool-range = < 60 80 >;
> - };
> -
> - spin_ctrl_1: motor-1 {
> - compatible = "ot,ferris-wheel-motor";
> - spin = "clockwise";
> - rpm_avail = < 50 >;
> - };
> -
> - spin_ctrl_2: motor-8 {
> - compatible = "ot,roller-coaster-motor";
> - };
> - };
> -
> - rides_1: fairway-1 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - compatible = "ot,rides";
> - status = "disabled";
> - orientation = < 127 >;
> -
> - ride@100 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - compatible = "ot,roller-coaster";
> - reg = < 0x00000100 0x100 >;
> - hvac-provider = < &hvac_1 >;
> - hvac-thermostat = < 29 > ;
> - hvac-zones = < 14 >;
> - hvac-zone-names = "operator";
> - spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
> - spin-controller-names = "track_1", "track_2";
> - queues = < 2 >;
> -
> - track@30 {
> - reg = < 0x00000030 0x10 >;
> - };
> -
> - track@40 {
> - reg = < 0x00000040 0x10 >;
> - };
> -
> - };
> - };
> -
> - lights_1: lights@30000 {
> - compatible = "ot,work-lights";
> - reg = < 0x00030000 0x1000 >;
> - status = "disabled";
> - };
> -
> - lights_2: lights@40000 {
> - compatible = "ot,show-lights";
> - reg = < 0x00040000 0x1000 >;
> - status = "disabled";
> - rate = < 13 138 >;
> - };
> -
> - retail_1: vending@50000 {
> - reg = < 0x00050000 0x1000 >;
> - compatible = "ot,tickets";
> - status = "disabled";
> - };
> -
> - };
> -};
> -
> +#include "overlay_common.dtsi"
> diff --git a/drivers/of/unittest-data/overlay_common.dtsi b/drivers/of/unittest-data/overlay_common.dtsi
> new file mode 100644
> index 000000000000..08874a72556e
> --- /dev/null
> +++ b/drivers/of/unittest-data/overlay_common.dtsi
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Base device tree that overlays will be applied against.
> + *
> + * Do not add any properties in node "/".
> + * Do not add any nodes other than "/testcase-data-2" in node "/".
> + * Do not add anything that would result in dtc creating node "/__fixups__".
> + * dtc will create nodes "/__symbols__" and "/__local_fixups__".
> + */
> +
> +/ {
> + testcase-data-2 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + electric_1: substation@100 {
> + compatible = "ot,big-volts-control";
> + reg = < 0x00000100 0x100 >;
> + status = "disabled";
> +
> + hvac_1: hvac-medium-1 {
> + compatible = "ot,hvac-medium";
> + heat-range = < 50 75 >;
> + cool-range = < 60 80 >;
> + };
> +
> + spin_ctrl_1: motor-1 {
> + compatible = "ot,ferris-wheel-motor";
> + spin = "clockwise";
> + rpm_avail = < 50 >;
> + };
> +
> + spin_ctrl_2: motor-8 {
> + compatible = "ot,roller-coaster-motor";
> + };
> + };
> +
> + rides_1: fairway-1 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "ot,rides";
> + status = "disabled";
> + orientation = < 127 >;
> +
> + ride@100 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "ot,roller-coaster";
> + reg = < 0x00000100 0x100 >;
> + hvac-provider = < &hvac_1 >;
> + hvac-thermostat = < 29 > ;
> + hvac-zones = < 14 >;
> + hvac-zone-names = "operator";
> + spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
> + spin-controller-names = "track_1", "track_2";
> + queues = < 2 >;
> +
> + track@30 {
> + reg = < 0x00000030 0x10 >;
> + };
> +
> + track@40 {
> + reg = < 0x00000040 0x10 >;
> + };
> +
> + };
> + };
> +
> + lights_1: lights@30000 {
> + compatible = "ot,work-lights";
> + reg = < 0x00030000 0x1000 >;
> + status = "disabled";
> + };
> +
> + lights_2: lights@40000 {
> + compatible = "ot,show-lights";
> + reg = < 0x00040000 0x1000 >;
> + status = "disabled";
> + rate = < 13 138 >;
> + };
> +
> + retail_1: vending@50000 {
> + reg = < 0x00050000 0x1000 >;
> + compatible = "ot,tickets";
> + status = "disabled";
> + };
> +
> + };
> +};
> +
> diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
> index a85b5e1c381a..04b9e7bb30d9 100644
> --- a/drivers/of/unittest-data/testcases.dts
> +++ b/drivers/of/unittest-data/testcases.dts
> @@ -2,19 +2,15 @@
> /dts-v1/;
> /plugin/;
>
> +#include "testcases_common.dtsi"
> +
> / {

Please add:

/*
* testcase data that intentionally results in an error
* is located here instead of in testcases_common.dtsi
* so that the static overlay apply tests will not include
* the error
*/

> testcase-data {
> - changeset {
> - prop-update = "hello";
> - prop-remove = "world";
> - node-remove {
> - };
> + testcase-device2 {
> + compatible = "testcase-device";
> + interrupt-parent = <&test_intc2>;
> + interrupts = <1>; /* invalid specifier - too short */
> };
> };
> +
> };
> -#include "tests-phandle.dtsi"
> -#include "tests-interrupts.dtsi"
> -#include "tests-match.dtsi"
> -#include "tests-address.dtsi"
> -#include "tests-platform.dtsi"
> -#include "tests-overlay.dtsi"
> diff --git a/drivers/of/unittest-data/testcases_common.dtsi b/drivers/of/unittest-data/testcases_common.dtsi
> new file mode 100644
> index 000000000000..19292bbb4cbb
> --- /dev/null
> +++ b/drivers/of/unittest-data/testcases_common.dtsi
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/ {
> + testcase-data {
> + changeset {
> + prop-update = "hello";
> + prop-remove = "world";
> + node-remove {
> + };
> + };
> + };
> +};
> +
> +#include "tests-phandle.dtsi"
> +#include "tests-interrupts.dtsi"
> +#include "tests-match.dtsi"
> +#include "tests-address.dtsi"
> +#include "tests-platform.dtsi"
> +#include "tests-overlay.dtsi"
> diff --git a/drivers/of/unittest-data/tests-interrupts.dtsi b/drivers/of/unittest-data/tests-interrupts.dtsi
> index ec175e800725..0e5914611107 100644
> --- a/drivers/of/unittest-data/tests-interrupts.dtsi
> +++ b/drivers/of/unittest-data/tests-interrupts.dtsi
> @@ -61,12 +61,5 @@ testcase-device1 {
> interrupt-parent = <&test_intc0>;
> interrupts = <1>;
> };

Please add:

/*
* testcase data that intentionally results in an error
* is located in testcases.dts instead of in this file
* so that the static overlay apply tests will not include
* the error
*/

> -
> - testcase-device2 {
> - compatible = "testcase-device";
> - interrupt-parent = <&test_intc2>;
> - interrupts = <1>; /* invalid specifier - too short */
> - };
> };
> -
> };
>

2021-02-19 05:24:16

by Frank Rowand

Subject: Re: [PATCH V7 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi

Hi Viresh,

I am in the wrong version with the comments below. You are at version 8 now.

-Frank

On 2/18/21 3:02 PM, Frank Rowand wrote:
> On 1/29/21 1:24 AM, Viresh Kumar wrote:
>> In order to build-test the same unit-test files using fdtoverlay tool,
>> move the device nodes from the existing overlay_base.dts and
>> testcases_common.dts files to .dtsi counterparts. The .dts files now
>> include the new .dtsi files, resulting in exactly the same behavior as
>> earlier.
>>
>> The .dtsi files can now be reused for compile time tests using
>> fdtoverlay (will be done by a later commit).
>>
>> This is required because the base files passed to fdtoverlay tool
>> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
>> tag).
>>
>> Note that this commit also moves "testcase-device2" node to
>> testcases.dts from tests-interrupts.dtsi, as this node has a deliberate
>> error in it and is only relevant for runtime testing done with
>> unittest.c.
>>
>> Signed-off-by: Viresh Kumar <[email protected]>
>> ---
>> drivers/of/unittest-data/overlay_base.dts | 90 +-----------------
>> drivers/of/unittest-data/overlay_common.dtsi | 91 +++++++++++++++++++
>> drivers/of/unittest-data/testcases.dts | 18 ++--
>> .../of/unittest-data/testcases_common.dtsi | 19 ++++
>> .../of/unittest-data/tests-interrupts.dtsi | 7 --
>> 5 files changed, 118 insertions(+), 107 deletions(-)
>> create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
>> create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
>>
>> diff --git a/drivers/of/unittest-data/overlay_base.dts b/drivers/of/unittest-data/overlay_base.dts
>> index 99ab9d12d00b..ab9014589c5d 100644
>> --- a/drivers/of/unittest-data/overlay_base.dts
>> +++ b/drivers/of/unittest-data/overlay_base.dts
>> @@ -2,92 +2,4 @@
>> /dts-v1/;
>> /plugin/;
>>
>> -/*
>> - * Base device tree that overlays will be applied against.
>> - *
>> - * Do not add any properties in node "/".
>> - * Do not add any nodes other than "/testcase-data-2" in node "/".
>> - * Do not add anything that would result in dtc creating node "/__fixups__".
>> - * dtc will create nodes "/__symbols__" and "/__local_fixups__".
>> - */
>> -
>> -/ {
>> - testcase-data-2 {
>> - #address-cells = <1>;
>> - #size-cells = <1>;
>> -
>> - electric_1: substation@100 {
>> - compatible = "ot,big-volts-control";
>> - reg = < 0x00000100 0x100 >;
>> - status = "disabled";
>> -
>> - hvac_1: hvac-medium-1 {
>> - compatible = "ot,hvac-medium";
>> - heat-range = < 50 75 >;
>> - cool-range = < 60 80 >;
>> - };
>> -
>> - spin_ctrl_1: motor-1 {
>> - compatible = "ot,ferris-wheel-motor";
>> - spin = "clockwise";
>> - rpm_avail = < 50 >;
>> - };
>> -
>> - spin_ctrl_2: motor-8 {
>> - compatible = "ot,roller-coaster-motor";
>> - };
>> - };
>> -
>> - rides_1: fairway-1 {
>> - #address-cells = <1>;
>> - #size-cells = <1>;
>> - compatible = "ot,rides";
>> - status = "disabled";
>> - orientation = < 127 >;
>> -
>> - ride@100 {
>> - #address-cells = <1>;
>> - #size-cells = <1>;
>> - compatible = "ot,roller-coaster";
>> - reg = < 0x00000100 0x100 >;
>> - hvac-provider = < &hvac_1 >;
>> - hvac-thermostat = < 29 > ;
>> - hvac-zones = < 14 >;
>> - hvac-zone-names = "operator";
>> - spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
>> - spin-controller-names = "track_1", "track_2";
>> - queues = < 2 >;
>> -
>> - track@30 {
>> - reg = < 0x00000030 0x10 >;
>> - };
>> -
>> - track@40 {
>> - reg = < 0x00000040 0x10 >;
>> - };
>> -
>> - };
>> - };
>> -
>> - lights_1: lights@30000 {
>> - compatible = "ot,work-lights";
>> - reg = < 0x00030000 0x1000 >;
>> - status = "disabled";
>> - };
>> -
>> - lights_2: lights@40000 {
>> - compatible = "ot,show-lights";
>> - reg = < 0x00040000 0x1000 >;
>> - status = "disabled";
>> - rate = < 13 138 >;
>> - };
>> -
>> - retail_1: vending@50000 {
>> - reg = < 0x00050000 0x1000 >;
>> - compatible = "ot,tickets";
>> - status = "disabled";
>> - };
>> -
>> - };
>> -};
>> -
>> +#include "overlay_common.dtsi"
>> diff --git a/drivers/of/unittest-data/overlay_common.dtsi b/drivers/of/unittest-data/overlay_common.dtsi
>> new file mode 100644
>> index 000000000000..08874a72556e
>> --- /dev/null
>> +++ b/drivers/of/unittest-data/overlay_common.dtsi
>> @@ -0,0 +1,91 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/*
>> + * Base device tree that overlays will be applied against.
>> + *
>> + * Do not add any properties in node "/".
>> + * Do not add any nodes other than "/testcase-data-2" in node "/".
>> + * Do not add anything that would result in dtc creating node "/__fixups__".
>> + * dtc will create nodes "/__symbols__" and "/__local_fixups__".
>> + */
>> +
>> +/ {
>> + testcase-data-2 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + electric_1: substation@100 {
>> + compatible = "ot,big-volts-control";
>> + reg = < 0x00000100 0x100 >;
>> + status = "disabled";
>> +
>> + hvac_1: hvac-medium-1 {
>> + compatible = "ot,hvac-medium";
>> + heat-range = < 50 75 >;
>> + cool-range = < 60 80 >;
>> + };
>> +
>> + spin_ctrl_1: motor-1 {
>> + compatible = "ot,ferris-wheel-motor";
>> + spin = "clockwise";
>> + rpm_avail = < 50 >;
>> + };
>> +
>> + spin_ctrl_2: motor-8 {
>> + compatible = "ot,roller-coaster-motor";
>> + };
>> + };
>> +
>> + rides_1: fairway-1 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + compatible = "ot,rides";
>> + status = "disabled";
>> + orientation = < 127 >;
>> +
>> + ride@100 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + compatible = "ot,roller-coaster";
>> + reg = < 0x00000100 0x100 >;
>> + hvac-provider = < &hvac_1 >;
>> + hvac-thermostat = < 29 > ;
>> + hvac-zones = < 14 >;
>> + hvac-zone-names = "operator";
>> + spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
>> + spin-controller-names = "track_1", "track_2";
>> + queues = < 2 >;
>> +
>> + track@30 {
>> + reg = < 0x00000030 0x10 >;
>> + };
>> +
>> + track@40 {
>> + reg = < 0x00000040 0x10 >;
>> + };
>> +
>> + };
>> + };
>> +
>> + lights_1: lights@30000 {
>> + compatible = "ot,work-lights";
>> + reg = < 0x00030000 0x1000 >;
>> + status = "disabled";
>> + };
>> +
>> + lights_2: lights@40000 {
>> + compatible = "ot,show-lights";
>> + reg = < 0x00040000 0x1000 >;
>> + status = "disabled";
>> + rate = < 13 138 >;
>> + };
>> +
>> + retail_1: vending@50000 {
>> + reg = < 0x00050000 0x1000 >;
>> + compatible = "ot,tickets";
>> + status = "disabled";
>> + };
>> +
>> + };
>> +};
>> +
>> diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
>> index a85b5e1c381a..04b9e7bb30d9 100644
>> --- a/drivers/of/unittest-data/testcases.dts
>> +++ b/drivers/of/unittest-data/testcases.dts
>> @@ -2,19 +2,15 @@
>> /dts-v1/;
>> /plugin/;
>>
>> +#include "testcases_common.dtsi"
>> +
>> / {
>
> Please add:
>
> /*
> * testcase data that intentionally results in an error
> * is located here instead of in testcases_common.dtsi
> * so that the static overlay apply tests will not include
> * the error
> */
>
>> testcase-data {
>> - changeset {
>> - prop-update = "hello";
>> - prop-remove = "world";
>> - node-remove {
>> - };
>> + testcase-device2 {
>> + compatible = "testcase-device";
>> + interrupt-parent = <&test_intc2>;
>> + interrupts = <1>; /* invalid specifier - too short */
>> };
>> };
>> +
>> };
>> -#include "tests-phandle.dtsi"
>> -#include "tests-interrupts.dtsi"
>> -#include "tests-match.dtsi"
>> -#include "tests-address.dtsi"
>> -#include "tests-platform.dtsi"
>> -#include "tests-overlay.dtsi"
>> diff --git a/drivers/of/unittest-data/testcases_common.dtsi b/drivers/of/unittest-data/testcases_common.dtsi
>> new file mode 100644
>> index 000000000000..19292bbb4cbb
>> --- /dev/null
>> +++ b/drivers/of/unittest-data/testcases_common.dtsi
>> @@ -0,0 +1,19 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/ {
>> + testcase-data {
>> + changeset {
>> + prop-update = "hello";
>> + prop-remove = "world";
>> + node-remove {
>> + };
>> + };
>> + };
>> +};
>> +
>> +#include "tests-phandle.dtsi"
>> +#include "tests-interrupts.dtsi"
>> +#include "tests-match.dtsi"
>> +#include "tests-address.dtsi"
>> +#include "tests-platform.dtsi"
>> +#include "tests-overlay.dtsi"
>> diff --git a/drivers/of/unittest-data/tests-interrupts.dtsi b/drivers/of/unittest-data/tests-interrupts.dtsi
>> index ec175e800725..0e5914611107 100644
>> --- a/drivers/of/unittest-data/tests-interrupts.dtsi
>> +++ b/drivers/of/unittest-data/tests-interrupts.dtsi
>> @@ -61,12 +61,5 @@ testcase-device1 {
>> interrupt-parent = <&test_intc0>;
>> interrupts = <1>;
>> };
>
> Please add:
>
> /*
> * testcase data that intentionally results in an error
> * is located in testcases.dts instead of in this file
> * so that the static overlay apply tests will not include
> * the error
> */
>
>> -
>> - testcase-device2 {
>> - compatible = "testcase-device";
>> - interrupt-parent = <&test_intc2>;
>> - interrupts = <1>; /* invalid specifier - too short */
>> - };
>> };
>> -
>> };
>>
>

2021-02-19 05:31:39

by Viresh Kumar

Subject: Re: [PATCH V7 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi

On 18-02-21, 23:20, Frank Rowand wrote:
> Hi Viresh,
>
> I am in the wrong version with the comments below. You are at version 8 now.

Yeah, it is fine. I have updated the patches already based on your
comments.

--
viresh

2021-02-25 05:50:33

by Frank Rowand

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

On 2/5/21 3:08 PM, Rob Herring wrote:
> On Fri, Feb 05, 2021 at 11:17:10AM +0100, Geert Uytterhoeven wrote:
>> Hi Viresh,
>>
>> On Fri, Feb 5, 2021 at 10:55 AM Viresh Kumar <[email protected]> wrote:
>>> On 05-02-21, 10:41, Geert Uytterhoeven wrote:
>>>> On Fri, Feb 5, 2021 at 10:25 AM Viresh Kumar <[email protected]> wrote:
>>>>> On 05-02-21, 10:02, Geert Uytterhoeven wrote:
>>>>>> Thanks for your patch
>>>>>> (which I only noticed because it appeared in dt-rh/for-next ;-)
>>>>>>
>>>>>> On Fri, Jan 29, 2021 at 8:31 AM Viresh Kumar <[email protected]> wrote:
>>>>>>> Add support for building DT overlays (%.dtbo). The overlay's source file
>>>>>>> will have the usual extension, i.e. .dts, though the blob will have
>>>>>>
>>>>>> Why use .dts and not .dtso for overlays?
>>>>>> Because you originally (until v5) had a single rule for building .dtb
>>>>>> and .dtbo files?
>>>>>
>>>>> I am fine with doing that as well if Rob and David agree to it. Rob
>>>>> did suggest that at one point but we didn't do much about it later on
>>>>> for some reason.
>>>>>
>>>>> FWIW, this will also require a change in the DTC compiler.
>>>>
>>>> Care to explain why? I've been using .dtsi for ages in
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/renesas-overlays
>>>
>>> I don't see you building them anywhere, they aren't added to the
>>> Makefile ever. What am I missing ?
>>>
>>> actually none of the dtso's were added to any makefile in that branch.
>>
>> E.g. "ARM: dts: Build all overlays if OF_OVERLAY=y"?
>> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=597ee90971687a45678cca8d16bf624d174a99eb
>>
>>> Anyway, the DTC needs to know how to treat the dtso format and it will
>>> error out currently with unknown format kind of errors.
>>>
>>> Below email [1] have some information on the kind of changes required
>>> here. Also note that we had to do similar changes for dtbo earlier
>>> [2].
>>>
>>> --
>>> viresh
>>>
>>> [1] https://lore.kernel.org/lkml/CAK7LNASViCOTGR7yDTfh0O+PAu+X-P2NwdY4oPMuXrr51awafA@mail.gmail.com/
>>
>> -@ is handled by "kbuild: Enable DT symbols when CONFIG_OF_OVERLAY is used"
>> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays&id=91e9d998514f3743125a707013a30d5f83054579
>>
>>> [2] https://lore.kernel.org/lkml/30fd0e5f2156665c713cf191c5fea9a5548360c0.1609926856.git.viresh.kumar@linaro.org/
>>
>> I never had a need for those changes to dtc. .dtso/.dtbo work fine regardless.
>
> I think what Viresh means is dtc won't automatically set the input type
> to dts if not .dts.
>
> We stuck with .dtbo as it's already widely used. I don't know about
> dtso though. If there's strong consensus to use .dtso, then let's do
> that. Whatever color for this shed you like.

I overlooked this and mistakenly thought that the move to .dtbo also
involved changing to .dtso. My bad.

My favorite color here is to use .dtso for the source file that will
be compiled to create a .dtbo.

Linus has already accepted patch 4/6 to 5.12-rc1, so changing to .dtso
will require another patch.

-Frank

>
> Rob
>

2021-03-04 08:24:33

by Viresh Kumar

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

On 24-02-21, 19:32, Frank Rowand wrote:
> I overlooked this and mistakenly thought that the move to .dtbo also
> involved changing to .dtso. My bad.
>
> My favorite color here is to use .dtso for the source file that will
> be compiled to create a .dtbo.
>
> Linus has already accepted patch 4/6 to 5.12-rc1, so changing to .dtso
> will require another patch.

Looks like this is what many people desire, lets do it and make it a
standard even if it wasn't followed earlier.

What about this ?

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index c430fbb36763..0dbedb61835f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -337,7 +337,7 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE

quiet_cmd_dtc = DTC $@
cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
- $(DTC) -O $(patsubst .%,%,$(suffix $@)) -o $@ -b 0 \
+ $(DTC) -I dts -O $(patsubst .%,%,$(suffix $@)) -o $@ -b 0 \
$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
-d $(depfile).dtc.tmp $(dtc-tmp) ; \
cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
@@ -348,6 +348,9 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
$(call if_changed_dep,dtc)

+$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
+ $(call if_changed_dep,dtc)
+
overlay-y := $(addprefix $(obj)/, $(overlay-y))

quiet_cmd_fdtoverlay = DTOVL $@
@@ -373,6 +376,9 @@ endef
$(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
$(call if_changed_rule,dtc,yaml)

+$(obj)/%.dt.yaml: $(src)/%.dtso $(DTC) $(DT_TMP_SCHEMA) FORCE
+ $(call if_changed_rule,dtc,yaml)
+
dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)

# Bzip2

-------------------------8<-------------------------

I had to keep the original line as is:

$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE

to support the unittest stuff as there are no dtso files there. There
are few things we can do here:

- Don't follow the dtso/dtbo convention for unittest, build files as
dtb only and everything will continue to work I suppose as
fdtoverlay won't complain.

- Keep the above line in Makefile, this doesn't sound right, isn't it
?

- Make .dts links for unittest file, maybe from the Makefile itself.

- Something else ?

--
viresh

2021-03-04 10:03:17

by Viresh Kumar

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

On 03-03-21, 11:44, Geert Uytterhoeven wrote:
> Hi Viresh,
>
> On Wed, Mar 3, 2021 at 6:21 AM Viresh Kumar <[email protected]> wrote:
> > On 24-02-21, 19:32, Frank Rowand wrote:
> > > I overlooked this and mistakenly thought that the move to .dtbo also
> > > involved changing to .dtso. My bad.
> > >
> > > My favorite color here is to use .dtso for the source file that will
> > > be compiled to create a .dtbo.
> > >
> > > Linus has already accepted patch 4/6 to 5.12-rc1, so changing to .dtso
> > > will require another patch.
> >
> > Looks like this is what many people desire, lets do it and make it a
> > standard even if it wasn't followed earlier.
> >
> > What about this ?
>
> Thanks, looks good to me, and works for me, so
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> Tested-by: Geert Uytterhoeven <[email protected]>

Thanks.

> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -337,7 +337,7 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
> >
> > quiet_cmd_dtc = DTC $@
> > cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> > - $(DTC) -O $(patsubst .%,%,$(suffix $@)) -o $@ -b 0 \
> > + $(DTC) -I dts -O $(patsubst .%,%,$(suffix $@)) -o $@ -b 0 \
> > $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
> > -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> > cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
> > @@ -348,6 +348,9 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> > $(call if_changed_dep,dtc)
> >
> > +$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
> > + $(call if_changed_dep,dtc)
> > +
> > overlay-y := $(addprefix $(obj)/, $(overlay-y))
> >
> > quiet_cmd_fdtoverlay = DTOVL $@
> > @@ -373,6 +376,9 @@ endef
> > $(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
> > $(call if_changed_rule,dtc,yaml)
> >
> > +$(obj)/%.dt.yaml: $(src)/%.dtso $(DTC) $(DT_TMP_SCHEMA) FORCE
>
> I'm wondering if "dt.yaml" should be changed to "dto.yaml" (here and in
> the existing rule earlier in Makefile.lib), to avoid issues if both foo.dts and
> foo.dtso exist? Unlikely, but it might happen...

I will let Rob answer that :)

> > I had to keep the original line as is:
> >
> > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> >
> > to support the unittest stuff as there are no dtso files there. There
> > are few things we can do here:
> >
> > - Don't follow the dtso/dtbo convention for unittest, build files as
> > dtb only and everything will continue to work I suppose as
> > fdtoverlay won't complain.
> >
> > - Keep the above line in Makefile, this doesn't sound right, isn't it
> > ?
> >
> > - Make .dts links for unittest file, maybe from the Makefile itself.
> >
> > - Something else ?
>
> Rename unittest .dts files to .dtso where applicable?

They are used for some runtime tests, we are reusing them to do this
testing as well, so renaming them is out of the question I believe.

--
viresh

2021-03-04 23:17:47

by Geert Uytterhoeven

Subject: Re: [PATCH V7 4/6] kbuild: Add support to build overlays (%.dtbo)

Hi Viresh,

On Wed, Mar 3, 2021 at 6:21 AM Viresh Kumar <[email protected]> wrote:
> On 24-02-21, 19:32, Frank Rowand wrote:
> > I overlooked this and mistakenly thought that the move to .dtbo also
> > involved changing to .dtso. My bad.
> >
> > My favorite color here is to use .dtso for the source file that will
> > be compiled to create a .dtbo.
> >
> > Linus has already accepted patch 4/6 to 5.12-rc1, so changing to .dtso
> > will require another patch.
>
> Looks like this is what many people desire, lets do it and make it a
> standard even if it wasn't followed earlier.
>
> What about this ?

Thanks, looks good to me, and works for me, so
Reviewed-by: Geert Uytterhoeven <[email protected]>
Tested-by: Geert Uytterhoeven <[email protected]>

> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -337,7 +337,7 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>
> quiet_cmd_dtc = DTC $@
> cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
> - $(DTC) -O $(patsubst .%,%,$(suffix $@)) -o $@ -b 0 \
> + $(DTC) -I dts -O $(patsubst .%,%,$(suffix $@)) -o $@ -b 0 \
> $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
> -d $(depfile).dtc.tmp $(dtc-tmp) ; \
> cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
> @@ -348,6 +348,9 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> $(call if_changed_dep,dtc)
>
> +$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
> + $(call if_changed_dep,dtc)
> +
> overlay-y := $(addprefix $(obj)/, $(overlay-y))
>
> quiet_cmd_fdtoverlay = DTOVL $@
> @@ -373,6 +376,9 @@ endef
> $(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
> $(call if_changed_rule,dtc,yaml)
>
> +$(obj)/%.dt.yaml: $(src)/%.dtso $(DTC) $(DT_TMP_SCHEMA) FORCE

I'm wondering if "dt.yaml" should be changed to "dto.yaml" (here and in
the existing rule earlier in Makefile.lib), to avoid issues if both foo.dts and
foo.dtso exist? Unlikely, but it might happen...

> I had to keep the original line as is:
>
> $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
>
> to support the unittest stuff as there are no dtso files there. There
> are few things we can do here:
>
> - Don't follow the dtso/dtbo convention for unittest, build files as
> dtb only and everything will continue to work I suppose as
> fdtoverlay won't complain.
>
> - Keep the above line in Makefile, this doesn't sound right, isn't it
> ?
>
> - Make .dts links for unittest file, maybe from the Makefile itself.
>
> - Something else ?

Rename unittest .dts files to .dtso where applicable?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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