linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V6 0/6] dt: build overlays
@ 2021-01-22 10:50 Viresh Kumar
  2021-01-22 10:50 ` [PATCH V6 1/6] scripts: dtc: Fetch fdtoverlay.c from external DTC project Viresh Kumar
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-22 10:50 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada,
	Michal Marek
  Cc: Viresh Kumar, Vincent Guittot, linux-kernel, anmar.oueja,
	Bill Mills, David Gibson, devicetree, linux-kbuild

Hi Frank/Rob,

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

Also note that Frank has already shared his concerns towards the error
reporting done by fdtoverlay tool [2], and David said it is not that
straight forward to make such changes in fdtoverlay. I have still
included the patch in this series for completeness.

FWIW, with fdtoverlay we generate a new build warning now, not sure why
though:

drivers/of/unittest-data/tests-interrupts.dtsi:20.5-28: Warning (interrupts_property): /testcase-data/testcase-device2:#interrupt-cells: size is (4), expected multiple of 8

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/74f8aa8f-ffab-3b0f-186f-31fb7395ebbb@gmail.com/

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             |  51 ++++++
 drivers/of/unittest-data/overlay_base.dts     |  90 +---------
 drivers/of/unittest-data/overlay_common.dtsi  |  91 ++++++++++
 drivers/of/unittest-data/static_base.dts      |   5 +
 drivers/of/unittest-data/testcases.dts        |  17 +-
 .../of/unittest-data/testcases_common.dtsi    |  18 ++
 scripts/Makefile.dtbinst                      |   3 +
 scripts/Makefile.lib                          |   5 +
 scripts/dtc/Makefile                          |   6 +-
 scripts/dtc/fdtdump.c                         | 163 ------------------
 scripts/dtc/update-dtc-source.sh              |   3 +-
 13 files changed, 187 insertions(+), 271 deletions(-)
 create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
 create mode 100644 drivers/of/unittest-data/static_base.dts
 create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
 delete mode 100644 scripts/dtc/fdtdump.c

-- 
2.25.0.rc1.19.g042ed3e048af


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH V6 1/6] scripts: dtc: Fetch fdtoverlay.c from external DTC project
  2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
@ 2021-01-22 10:50 ` Viresh Kumar
  2021-01-22 10:50 ` [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool Viresh Kumar
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-22 10:50 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Viresh Kumar, Vincent Guittot, linux-kernel, anmar.oueja,
	Bill Mills, David Gibson, devicetree, Michal Marek

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 <viresh.kumar@linaro.org>
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool
  2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
  2021-01-22 10:50 ` [PATCH V6 1/6] scripts: dtc: Fetch fdtoverlay.c from external DTC project Viresh Kumar
@ 2021-01-22 10:50 ` Viresh Kumar
  2021-01-23  0:35   ` David Gibson
  2021-01-29  6:03   ` Frank Rowand
  2021-01-22 10:50 ` [PATCH V6 3/6] scripts: dtc: Remove the unused fdtdump.c file Viresh Kumar
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-22 10:50 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Viresh Kumar, Vincent Guittot, linux-kernel, anmar.oueja,
	Bill Mills, David Gibson, devicetree, Michal Marek

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 (or merges) 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 <viresh.kumar@linaro.org>
---
 scripts/dtc/Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 4852bf44e913..5f19386a49eb 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -1,13 +1,17 @@
 # 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
 
+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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V6 3/6] scripts: dtc: Remove the unused fdtdump.c file
  2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
  2021-01-22 10:50 ` [PATCH V6 1/6] scripts: dtc: Fetch fdtoverlay.c from external DTC project Viresh Kumar
  2021-01-22 10:50 ` [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool Viresh Kumar
@ 2021-01-22 10:50 ` Viresh Kumar
  2021-01-22 10:50 ` [PATCH V6 4/6] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-22 10:50 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Viresh Kumar, Vincent Guittot, linux-kernel, anmar.oueja,
	Bill Mills, David Gibson, devicetree, Michal Marek

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 <viresh.kumar@linaro.org>
---
 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V6 4/6] kbuild: Add support to build overlays (%.dtbo)
  2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
                   ` (2 preceding siblings ...)
  2021-01-22 10:50 ` [PATCH V6 3/6] scripts: dtc: Remove the unused fdtdump.c file Viresh Kumar
@ 2021-01-22 10:50 ` Viresh Kumar
  2021-01-22 11:12   ` Masahiro Yamada
  2021-01-22 10:50 ` [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi Viresh Kumar
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Viresh Kumar @ 2021-01-22 10:50 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada,
	Michal Marek
  Cc: Viresh Kumar, Vincent Guittot, linux-kernel, anmar.oueja,
	Bill Mills, David Gibson, devicetree, linux-kbuild

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.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .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 9e73f82e0d86..18fd02f55d9b 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
                   ` (3 preceding siblings ...)
  2021-01-22 10:50 ` [PATCH V6 4/6] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
@ 2021-01-22 10:50 ` Viresh Kumar
  2021-01-23  3:07   ` David Gibson
  2021-01-27  5:56   ` Viresh Kumar
  2021-01-22 10:50 ` [PATCH V6 6/6] of: unittest: Statically apply overlays using fdtoverlay Viresh Kumar
  2021-01-26  3:45 ` [PATCH V6 0/6] dt: build overlays Frank Rowand
  6 siblings, 2 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-22 10:50 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Viresh Kumar, Vincent Guittot, linux-kernel, anmar.oueja,
	Bill Mills, David Gibson, devicetree, Michal Marek

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 files. 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 in a later patch).

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

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
 drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
 drivers/of/unittest-data/testcases.dts        | 17 +---
 .../of/unittest-data/testcases_common.dtsi    | 18 ++++
 4 files changed, 111 insertions(+), 105 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..185125085784 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -2,19 +2,4 @@
 /dts-v1/;
 /plugin/;
 
-/ {
-	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"
+#include "testcases_common.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..05f16606b6a5
--- /dev/null
+++ b/drivers/of/unittest-data/testcases_common.dtsi
@@ -0,0 +1,18 @@
+// 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"
-- 
2.25.0.rc1.19.g042ed3e048af


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH V6 6/6] of: unittest: Statically apply overlays using fdtoverlay
  2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
                   ` (4 preceding siblings ...)
  2021-01-22 10:50 ` [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi Viresh Kumar
@ 2021-01-22 10:50 ` Viresh Kumar
  2021-01-26  3:15   ` Frank Rowand
  2021-01-26  3:45 ` [PATCH V6 0/6] dt: build overlays Frank Rowand
  6 siblings, 1 reply; 21+ messages in thread
From: Viresh Kumar @ 2021-01-22 10:50 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Viresh Kumar, Vincent Guittot, linux-kernel, anmar.oueja,
	Bill Mills, David Gibson, devicetree, Michal Marek

Now that fdtoverlay is part of the kernel build, start using it to test
the unitest overlays we have by applying them statically. Create a new
base file static_base.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 in the static_test.dtb.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/of/unittest-data/Makefile        | 51 ++++++++++++++++++++++++
 drivers/of/unittest-data/static_base.dts |  5 +++
 2 files changed, 56 insertions(+)
 create mode 100644 drivers/of/unittest-data/static_base.dts

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 009f4045c8e4..586fa8cda916 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -34,7 +34,58 @@ DTC_FLAGS_overlay += -@
 DTC_FLAGS_overlay_bad_phandle += -@
 DTC_FLAGS_overlay_bad_symbol += -@
 DTC_FLAGS_overlay_base += -@
+DTC_FLAGS_static_base += -@
 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.dtb to create static_test.dtb
+# If fdtoverlay detects an error than the kernel build will fail.
+# static_test.dtb is 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 \
+#			overlay_base.dtb \
+
+apply_static_overlay := overlay.dtb \
+			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
+
+quiet_cmd_fdtoverlay = FDTOVERLAY $@
+      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^
+
+$(obj)/static_test.dtb: $(obj)/static_base.dtb $(addprefix $(obj)/,$(apply_static_overlay))
+	$(call if_changed,fdtoverlay)
+
+always-$(CONFIG_OF_OVERLAY) += static_test.dtb
diff --git a/drivers/of/unittest-data/static_base.dts b/drivers/of/unittest-data/static_base.dts
new file mode 100644
index 000000000000..3c9af4aefb96
--- /dev/null
+++ b/drivers/of/unittest-data/static_base.dts
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "overlay_common.dtsi"
+#include "testcases_common.dtsi"
-- 
2.25.0.rc1.19.g042ed3e048af


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 4/6] kbuild: Add support to build overlays (%.dtbo)
  2021-01-22 10:50 ` [PATCH V6 4/6] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
@ 2021-01-22 11:12   ` Masahiro Yamada
  0 siblings, 0 replies; 21+ messages in thread
From: Masahiro Yamada @ 2021-01-22 11:12 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Frank Rowand, Rob Herring, Pantelis Antoniou, Michal Marek,
	Vincent Guittot, Linux Kernel Mailing List, Anmar Oueja,
	Bill Mills, David Gibson, DTML, Linux Kbuild mailing list

On Fri, Jan 22, 2021 at 7:51 PM Viresh Kumar <viresh.kumar@linaro.org> 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
> .dtbo extension to distinguish it from normal blobs.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---

Acked-by: Masahiro Yamada <masahiroy@kernel.org>



>  .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 9e73f82e0d86..18fd02f55d9b 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
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool
  2021-01-22 10:50 ` [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool Viresh Kumar
@ 2021-01-23  0:35   ` David Gibson
  2021-01-25  3:15     ` Viresh Kumar
  2021-01-29  6:03   ` Frank Rowand
  1 sibling, 1 reply; 21+ messages in thread
From: David Gibson @ 2021-01-23  0:35 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada,
	Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	devicetree, Michal Marek

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

On Fri, Jan 22, 2021 at 04:20:32PM +0530, Viresh Kumar wrote:
> 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 (or merges) one or more overlay dtb

Saying "merges" here is probably misleading, since as I mentioned
elsewhere fdtoverlay can *not* merge overlays, only apply them to a
base tree.

> 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 <viresh.kumar@linaro.org>
> ---
>  scripts/dtc/Makefile | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index 4852bf44e913..5f19386a49eb 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -1,13 +1,17 @@
>  # 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
>  
> +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
>  

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  2021-01-22 10:50 ` [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi Viresh Kumar
@ 2021-01-23  3:07   ` David Gibson
  2021-01-25  3:16     ` Viresh Kumar
  2021-01-26  3:18     ` Frank Rowand
  2021-01-27  5:56   ` Viresh Kumar
  1 sibling, 2 replies; 21+ messages in thread
From: David Gibson @ 2021-01-23  3:07 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada,
	Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	devicetree, Michal Marek

[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]

On Fri, Jan 22, 2021 at 04:20:35PM +0530, 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 files. 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 in a later patch).
> 
> This is required because the base files passed to fdtoverlay tool
> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
> tag).
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
>  drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
>  drivers/of/unittest-data/testcases.dts        | 17 +---
>  .../of/unittest-data/testcases_common.dtsi    | 18 ++++
>  4 files changed, 111 insertions(+), 105 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/;

This still makes no sense to me.  Is this data intended as a base
tree, or as an overlay?  If it's an overlay, what are the constraints
on the base tree it's supposed to apply to.

This patch is treating it as both in different places, but that's such
a bizarre usecase it needs detailed justification.  It really looks
like the unittest stuff is doing some very bogus stuff that should be
fixed first, before trying to do this on top.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool
  2021-01-23  0:35   ` David Gibson
@ 2021-01-25  3:15     ` Viresh Kumar
  0 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-25  3:15 UTC (permalink / raw)
  To: David Gibson
  Cc: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada,
	Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	devicetree, Michal Marek

On 23-01-21, 11:35, David Gibson wrote:
> On Fri, Jan 22, 2021 at 04:20:32PM +0530, Viresh Kumar wrote:
> > 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 (or merges) one or more overlay dtb
> 
> Saying "merges" here is probably misleading, since as I mentioned
> elsewhere fdtoverlay can *not* merge overlays, only apply them to a
> base tree.

Right, my mistake.

-- 
viresh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  2021-01-23  3:07   ` David Gibson
@ 2021-01-25  3:16     ` Viresh Kumar
  2021-01-26  3:18     ` Frank Rowand
  1 sibling, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-25  3:16 UTC (permalink / raw)
  To: David Gibson
  Cc: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada,
	Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	devicetree, Michal Marek

On 23-01-21, 14:07, David Gibson wrote:
> On Fri, Jan 22, 2021 at 04:20:35PM +0530, 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 files. 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 in a later patch).
> > 
> > This is required because the base files passed to fdtoverlay tool
> > shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
> > tag).
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
> >  drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
> >  drivers/of/unittest-data/testcases.dts        | 17 +---
> >  .../of/unittest-data/testcases_common.dtsi    | 18 ++++
> >  4 files changed, 111 insertions(+), 105 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/;
> 
> This still makes no sense to me.  Is this data intended as a base
> tree, or as an overlay?  If it's an overlay, what are the constraints
> on the base tree it's supposed to apply to.
> 
> This patch is treating it as both in different places, but that's such
> a bizarre usecase it needs detailed justification.  It really looks
> like the unittest stuff is doing some very bogus stuff that should be
> fixed first, before trying to do this on top.

I will let Frank respond to that :)

-- 
viresh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 6/6] of: unittest: Statically apply overlays using fdtoverlay
  2021-01-22 10:50 ` [PATCH V6 6/6] of: unittest: Statically apply overlays using fdtoverlay Viresh Kumar
@ 2021-01-26  3:15   ` Frank Rowand
  2021-01-26  3:21     ` Frank Rowand
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Rowand @ 2021-01-26  3:15 UTC (permalink / raw)
  To: Viresh Kumar, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	David Gibson, devicetree, Michal Marek

On 1/22/21 4:50 AM, 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 a new
> base file static_base.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 in the static_test.dtb.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/of/unittest-data/Makefile        | 51 ++++++++++++++++++++++++
>  drivers/of/unittest-data/static_base.dts |  5 +++
>  2 files changed, 56 insertions(+)
>  create mode 100644 drivers/of/unittest-data/static_base.dts
> 
> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
> index 009f4045c8e4..586fa8cda916 100644
> --- a/drivers/of/unittest-data/Makefile
> +++ b/drivers/of/unittest-data/Makefile
> @@ -34,7 +34,58 @@ DTC_FLAGS_overlay += -@
>  DTC_FLAGS_overlay_bad_phandle += -@
>  DTC_FLAGS_overlay_bad_symbol += -@
>  DTC_FLAGS_overlay_base += -@
> +DTC_FLAGS_static_base += -@
>  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.dtb to create static_test.dtb
> +# If fdtoverlay detects an error than the kernel build will fail.
> +# static_test.dtb is 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 \
> +#			overlay_base.dtb \
> +
> +apply_static_overlay := overlay.dtb \

rename apply_static_overlay to apply_static_overlay_2:

   apply_static_overlay_2 := overlay.dtb

Then the remainder of apply_static_overlay becomes apply_static_overlay_1:

   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
> +
> +quiet_cmd_fdtoverlay = FDTOVERLAY $@
> +      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^
> +


> +$(obj)/static_test.dtb: $(obj)/static_base.dtb $(addprefix $(obj)/,$(apply_static_overlay))
> +	$(call if_changed,fdtoverlay)

Split the static_test.dtb into _1 and _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.dtb


> diff --git a/drivers/of/unittest-data/static_base.dts b/drivers/of/unittest-data/static_base.dts
> new file mode 100644
> index 000000000000..3c9af4aefb96
> --- /dev/null
> +++ b/drivers/of/unittest-data/static_base.dt> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +#include "overlay_common.dtsi"
> +#include "testcases_common.dtsi"
> 

Split static_base.dts into static_base_1.dts and static_base_2.dts:

static_base_1.dts:

> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +#include "testcases_common.dtsi"


static_base_2.dts:

> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +#include "overlay_common.dtsi"

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  2021-01-23  3:07   ` David Gibson
  2021-01-25  3:16     ` Viresh Kumar
@ 2021-01-26  3:18     ` Frank Rowand
  1 sibling, 0 replies; 21+ messages in thread
From: Frank Rowand @ 2021-01-26  3:18 UTC (permalink / raw)
  To: David Gibson, Viresh Kumar
  Cc: Rob Herring, Pantelis Antoniou, Masahiro Yamada, Vincent Guittot,
	linux-kernel, anmar.oueja, Bill Mills, devicetree, Michal Marek

On 1/22/21 9:07 PM, David Gibson wrote:
> On Fri, Jan 22, 2021 at 04:20:35PM +0530, 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 files. 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 in a later patch).
>>
>> This is required because the base files passed to fdtoverlay tool
>> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
>> tag).
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
>>  drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
>>  drivers/of/unittest-data/testcases.dts        | 17 +---
>>  .../of/unittest-data/testcases_common.dtsi    | 18 ++++
>>  4 files changed, 111 insertions(+), 105 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/;
> 
> This still makes no sense to me.  Is this data intended as a base
> tree, or as an overlay?  If it's an overlay, what are the constraints
> on the base tree it's supposed to apply to.

I have already replied several times that this should not make sense to
anyone unless they read unittest.c and see in detail how these FDTs are
abused.  I have stated several times that the usage is bizarre and not
normal.

> 
> This patch is treating it as both in different places, but that's such
> a bizarre usecase it needs detailed justification.  It really looks
> like the unittest stuff is doing some very bogus stuff that should be
> fixed first, before trying to do this on top.
> 

The unittest stuff is bizarre, but it is correct.  This patch series does
not alter the current usage.

-Frank

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 6/6] of: unittest: Statically apply overlays using fdtoverlay
  2021-01-26  3:15   ` Frank Rowand
@ 2021-01-26  3:21     ` Frank Rowand
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Rowand @ 2021-01-26  3:21 UTC (permalink / raw)
  To: Viresh Kumar, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	David Gibson, devicetree, Michal Marek

On 1/25/21 9:15 PM, Frank Rowand wrote:
> On 1/22/21 4:50 AM, 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 a new
>> base file static_base.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 in the static_test.dtb.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  drivers/of/unittest-data/Makefile        | 51 ++++++++++++++++++++++++
>>  drivers/of/unittest-data/static_base.dts |  5 +++
>>  2 files changed, 56 insertions(+)
>>  create mode 100644 drivers/of/unittest-data/static_base.dts
>>
>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
>> index 009f4045c8e4..586fa8cda916 100644
>> --- a/drivers/of/unittest-data/Makefile
>> +++ b/drivers/of/unittest-data/Makefile
>> @@ -34,7 +34,58 @@ DTC_FLAGS_overlay += -@
>>  DTC_FLAGS_overlay_bad_phandle += -@
>>  DTC_FLAGS_overlay_bad_symbol += -@
>>  DTC_FLAGS_overlay_base += -@
>> +DTC_FLAGS_static_base += -@
>>  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.dtb to create static_test.dtb
>> +# If fdtoverlay detects an error than the kernel build will fail.
>> +# static_test.dtb is 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 \
>> +#			overlay_base.dtb \
>> +
>> +apply_static_overlay := overlay.dtb \
> 
> rename apply_static_overlay to apply_static_overlay_2:
> 
>    apply_static_overlay_2 := overlay.dtb
> 
> Then the remainder of apply_static_overlay becomes apply_static_overlay_1:
> 
>    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
>> +
>> +quiet_cmd_fdtoverlay = FDTOVERLAY $@
>> +      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^
>> +
> 
> 
>> +$(obj)/static_test.dtb: $(obj)/static_base.dtb $(addprefix $(obj)/,$(apply_static_overlay))
>> +	$(call if_changed,fdtoverlay)
> 
> Split the static_test.dtb into _1 and _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.dtb

and of source the config line becomes static_test_1.dtb and static_test_2.dtb

Hopefully I haven't missed any other details...

-Frank

> 
> 
>> diff --git a/drivers/of/unittest-data/static_base.dts b/drivers/of/unittest-data/static_base.dts
>> new file mode 100644
>> index 000000000000..3c9af4aefb96
>> --- /dev/null
>> +++ b/drivers/of/unittest-data/static_base.dt> @@ -0,0 +1,5 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/dts-v1/;
>> +
>> +#include "overlay_common.dtsi"
>> +#include "testcases_common.dtsi"
>>
> 
> Split static_base.dts into static_base_1.dts and static_base_2.dts:
> 
> static_base_1.dts:
> 
>> +// SPDX-License-Identifier: GPL-2.0
>> +/dts-v1/;
>> +
>> +#include "testcases_common.dtsi"
> 
> 
> static_base_2.dts:
> 
>> +// SPDX-License-Identifier: GPL-2.0
>> +/dts-v1/;
>> +
>> +#include "overlay_common.dtsi"


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 0/6] dt: build overlays
  2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
                   ` (5 preceding siblings ...)
  2021-01-22 10:50 ` [PATCH V6 6/6] of: unittest: Statically apply overlays using fdtoverlay Viresh Kumar
@ 2021-01-26  3:45 ` Frank Rowand
  6 siblings, 0 replies; 21+ messages in thread
From: Frank Rowand @ 2021-01-26  3:45 UTC (permalink / raw)
  To: Viresh Kumar, Rob Herring, Pantelis Antoniou, Masahiro Yamada,
	Michal Marek
  Cc: Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	David Gibson, devicetree, linux-kbuild

On 1/22/21 4:50 AM, Viresh Kumar wrote:
> Hi Frank/Rob,
> 
> 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).
> 

> Also note that Frank has already shared his concerns towards the error
> reporting done by fdtoverlay tool [2], and David said it is not that
> straight forward to make such changes in fdtoverlay. I have still
> included the patch in this series for completeness.

I started to reply to this email with questions for David about how to
improve the fdtoverlay error reporting.  But then decided that instead
of trying to paraphrase the comments in v4 of this patch series, it
would be more efficient to ask in the v4 thread.  So my questions are
over there...

-Frank

> 
> FWIW, with fdtoverlay we generate a new build warning now, not sure why
> though:
> 
> drivers/of/unittest-data/tests-interrupts.dtsi:20.5-28: Warning (interrupts_property): /testcase-data/testcase-device2:#interrupt-cells: size is (4), expected multiple of 8
> 
> 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/74f8aa8f-ffab-3b0f-186f-31fb7395ebbb@gmail.com/
> 
> 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             |  51 ++++++
>  drivers/of/unittest-data/overlay_base.dts     |  90 +---------
>  drivers/of/unittest-data/overlay_common.dtsi  |  91 ++++++++++
>  drivers/of/unittest-data/static_base.dts      |   5 +
>  drivers/of/unittest-data/testcases.dts        |  17 +-
>  .../of/unittest-data/testcases_common.dtsi    |  18 ++
>  scripts/Makefile.dtbinst                      |   3 +
>  scripts/Makefile.lib                          |   5 +
>  scripts/dtc/Makefile                          |   6 +-
>  scripts/dtc/fdtdump.c                         | 163 ------------------
>  scripts/dtc/update-dtc-source.sh              |   3 +-
>  13 files changed, 187 insertions(+), 271 deletions(-)
>  create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
>  create mode 100644 drivers/of/unittest-data/static_base.dts
>  create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
>  delete mode 100644 scripts/dtc/fdtdump.c
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  2021-01-22 10:50 ` [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi Viresh Kumar
  2021-01-23  3:07   ` David Gibson
@ 2021-01-27  5:56   ` Viresh Kumar
  2021-01-29  5:59     ` Frank Rowand
  2021-01-29  6:39     ` Frank Rowand
  1 sibling, 2 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-27  5:56 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	David Gibson, devicetree, Michal Marek

On 22-01-21, 16:20, 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 files. 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 in a later patch).
> 
> This is required because the base files passed to fdtoverlay tool
> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
> tag).
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
>  drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
>  drivers/of/unittest-data/testcases.dts        | 17 +---
>  .../of/unittest-data/testcases_common.dtsi    | 18 ++++
>  4 files changed, 111 insertions(+), 105 deletions(-)
>  create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
>  create mode 100644 drivers/of/unittest-data/testcases_common.dtsi

Frank,

As I mentioned in the cover-letter, I get a build warning right now:

drivers/of/unittest-data/tests-interrupts.dtsi:20.5-28: Warning (interrupts_property): /testcase-data/testcase-device2:#interrupt-cells: size is (4), expected multiple of 8

I think I need to add below diff to this patch to fix this warning, will that
be okay ?

diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
index 185125085784..04b9e7bb30d9 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -3,3 +3,14 @@
 /plugin/;
 
 #include "testcases_common.dtsi"
+
+/ {
+       testcase-data {
+               testcase-device2 {
+                       compatible = "testcase-device";
+                       interrupt-parent = <&test_intc2>;
+                       interrupts = <1>; /* invalid specifier - too short */
+               };
+       };
+
+};
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 */
-               };
        };
-
 };

-- 
viresh

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  2021-01-27  5:56   ` Viresh Kumar
@ 2021-01-29  5:59     ` Frank Rowand
  2021-01-29  6:39     ` Frank Rowand
  1 sibling, 0 replies; 21+ messages in thread
From: Frank Rowand @ 2021-01-29  5:59 UTC (permalink / raw)
  To: Viresh Kumar, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	David Gibson, devicetree, Michal Marek

Hi Viresh,

On 1/26/21 11:56 PM, Viresh Kumar wrote:
> On 22-01-21, 16:20, 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 files. 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 in a later patch).
>>
>> This is required because the base files passed to fdtoverlay tool
>> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
>> tag).
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
>>  drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
>>  drivers/of/unittest-data/testcases.dts        | 17 +---
>>  .../of/unittest-data/testcases_common.dtsi    | 18 ++++
>>  4 files changed, 111 insertions(+), 105 deletions(-)
>>  create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
>>  create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
> 
> Frank,
> 
> As I mentioned in the cover-letter, I get a build warning right now:
> 
> drivers/of/unittest-data/tests-interrupts.dtsi:20.5-28: Warning (interrupts_property): /testcase-data/testcase-device2:#interrupt-cells: size is (4), expected multiple of 8

Thanks for catching that.

> 
> I think I need to add below diff to this patch to fix this warning, will that
> be okay ?

Nope, the change below won't work because it removes the node testcase-device2 from the tests
that unittest.c does (if I am thinking correctly).  I will double check my thinking, but I
know you are spinning the patch, so I didn't want to delay this reply.

Note that this node has a deliberate error in it "/* invalid specifier - too short */".

I'm not sure why the dtc warning triggers on line 20 instead of line 68.  I'll have to go
look at the dtc source to better understand the warning.

-Frank

> 
> diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
> index 185125085784..04b9e7bb30d9 100644
> --- a/drivers/of/unittest-data/testcases.dts
> +++ b/drivers/of/unittest-data/testcases.dts
> @@ -3,3 +3,14 @@
>  /plugin/;
>  
>  #include "testcases_common.dtsi"
> +
> +/ {
> +       testcase-data {
> +               testcase-device2 {
> +                       compatible = "testcase-device";
> +                       interrupt-parent = <&test_intc2>;
> +                       interrupts = <1>; /* invalid specifier - too short */
> +               };
> +       };
> +
> +};
> 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 */
> -               };
>         };
> -
>  };
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool
  2021-01-22 10:50 ` [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool Viresh Kumar
  2021-01-23  0:35   ` David Gibson
@ 2021-01-29  6:03   ` Frank Rowand
  2021-01-29  6:11     ` Viresh Kumar
  1 sibling, 1 reply; 21+ messages in thread
From: Frank Rowand @ 2021-01-29  6:03 UTC (permalink / raw)
  To: Viresh Kumar, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	David Gibson, devicetree, Michal Marek

On 1/22/21 4:50 AM, Viresh Kumar wrote:
> 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 (or merges) 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 <viresh.kumar@linaro.org>
> ---
>  scripts/dtc/Makefile | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index 4852bf44e913..5f19386a49eb 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -1,13 +1,17 @@
>  # 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
> 

Please add this comment:

# The upstream project builds libfdt as a separate library.  We are choosing to
# instead directly link the libfdt object files into fdtoverly

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool
  2021-01-29  6:03   ` Frank Rowand
@ 2021-01-29  6:11     ` Viresh Kumar
  0 siblings, 0 replies; 21+ messages in thread
From: Viresh Kumar @ 2021-01-29  6:11 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Rob Herring, Pantelis Antoniou, Masahiro Yamada, Vincent Guittot,
	linux-kernel, anmar.oueja, Bill Mills, David Gibson, devicetree,
	Michal Marek

On 29-01-21, 00:03, Frank Rowand wrote:
> On 1/22/21 4:50 AM, Viresh Kumar wrote:
> > 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 (or merges) 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 <viresh.kumar@linaro.org>
> > ---
> >  scripts/dtc/Makefile | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> > index 4852bf44e913..5f19386a49eb 100644
> > --- a/scripts/dtc/Makefile
> > +++ b/scripts/dtc/Makefile
> > @@ -1,13 +1,17 @@
> >  # 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
> > 
> 
> Please add this comment:
> 
> # The upstream project builds libfdt as a separate library.  We are choosing to
> # instead directly link the libfdt object files into fdtoverly

My bad, I checked this again and you gave the exact same comment
during V4 as well. Sorry about missing this earlier.

-- 
viresh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  2021-01-27  5:56   ` Viresh Kumar
  2021-01-29  5:59     ` Frank Rowand
@ 2021-01-29  6:39     ` Frank Rowand
  1 sibling, 0 replies; 21+ messages in thread
From: Frank Rowand @ 2021-01-29  6:39 UTC (permalink / raw)
  To: Viresh Kumar, Rob Herring, Pantelis Antoniou, Masahiro Yamada
  Cc: Vincent Guittot, linux-kernel, anmar.oueja, Bill Mills,
	David Gibson, devicetree, Michal Marek

Hi Viresh,

Second attempt, I think the first reply did not properly send.

On 1/26/21 11:56 PM, Viresh Kumar wrote:
> On 22-01-21, 16:20, 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 files. 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 in a later patch).
>>
>> This is required because the base files passed to fdtoverlay tool
>> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
>> tag).
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
>>  drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
>>  drivers/of/unittest-data/testcases.dts        | 17 +---
>>  .../of/unittest-data/testcases_common.dtsi    | 18 ++++
>>  4 files changed, 111 insertions(+), 105 deletions(-)
>>  create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
>>  create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
> 
> Frank,
> 
> As I mentioned in the cover-letter, I get a build warning right now:
> 
> drivers/of/unittest-data/tests-interrupts.dtsi:20.5-28: Warning (interrupts_property): /testcase-data/testcase-device2:#interrupt-cells: size is (4), expected multiple of 8

Thanks for catching that.

> 
> I think I need to add below diff to this patch to fix this warning, will that
> be okay ?

In my first reply, I said "nope", or something to that effect.  Upon reflection, it looks
like the below diff will fix the problem.  This is base on source code inspection and
building with the diff applied.

I did not successfully boot my target (I have some issues to resolve after updating
the OS on my development host), so I have not verified that unittest is not impacted.

-Frank

> 
> diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
> index 185125085784..04b9e7bb30d9 100644
> --- a/drivers/of/unittest-data/testcases.dts
> +++ b/drivers/of/unittest-data/testcases.dts
> @@ -3,3 +3,14 @@
>  /plugin/;
>  
>  #include "testcases_common.dtsi"
> +
> +/ {
> +       testcase-data {
> +               testcase-device2 {
> +                       compatible = "testcase-device";
> +                       interrupt-parent = <&test_intc2>;
> +                       interrupts = <1>; /* invalid specifier - too short */
> +               };
> +       };
> +
> +};
> 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 */
> -               };
>         };
> -
>  };
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-01-29  6:40 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 10:50 [PATCH V6 0/6] dt: build overlays Viresh Kumar
2021-01-22 10:50 ` [PATCH V6 1/6] scripts: dtc: Fetch fdtoverlay.c from external DTC project Viresh Kumar
2021-01-22 10:50 ` [PATCH V6 2/6] scripts: dtc: Build fdtoverlay tool Viresh Kumar
2021-01-23  0:35   ` David Gibson
2021-01-25  3:15     ` Viresh Kumar
2021-01-29  6:03   ` Frank Rowand
2021-01-29  6:11     ` Viresh Kumar
2021-01-22 10:50 ` [PATCH V6 3/6] scripts: dtc: Remove the unused fdtdump.c file Viresh Kumar
2021-01-22 10:50 ` [PATCH V6 4/6] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
2021-01-22 11:12   ` Masahiro Yamada
2021-01-22 10:50 ` [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi Viresh Kumar
2021-01-23  3:07   ` David Gibson
2021-01-25  3:16     ` Viresh Kumar
2021-01-26  3:18     ` Frank Rowand
2021-01-27  5:56   ` Viresh Kumar
2021-01-29  5:59     ` Frank Rowand
2021-01-29  6:39     ` Frank Rowand
2021-01-22 10:50 ` [PATCH V6 6/6] of: unittest: Statically apply overlays using fdtoverlay Viresh Kumar
2021-01-26  3:15   ` Frank Rowand
2021-01-26  3:21     ` Frank Rowand
2021-01-26  3:45 ` [PATCH V6 0/6] dt: build overlays Frank Rowand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).