dt: Add fdtoverlay rule and statically build unittest

PATCH Applied Viresh Kumar 6 participants 36 messages Mar 10, 2021 – Apr 9, 2021

Subsystems (2)

Kbuild Device tree

lore

Share & tools

lore

Viresh Kumar

author

2021-03-10 05:36 UTC #0 lore raw

v11 cover dt: Add fdtoverlay rule and statically build unittest

Hi,

This patchset adds a generic rule for applying overlays using fdtoverlay tool and then updates unittests to get built statically using the same.

V10->V11:
- Update patch 4/5 to fix checkpatch warning on spaces and tabs.
- Added Acked-by from Masahiro for patch 2/5.

V9->V10:
- Add a new patch to allow .dtso files.
- Update 2/5 to be more efficient and also generate symbols for base files automatically.
- No need to add lines like DTC_FLAGS_foo_base += -@ in patch 5/5.
- Add Ack by Masahiro for 1/5.

V8->V9:
- Added some comment in patch 3/4 based on Frank's suggestions.

V7->V8:
- Patch 1 is new.
- Platforms need to use dtb-y += foo.dtb instead of overlay-y += foo.dtb.
- Use multi_depend instead of .SECONDEXPANSION.
- Use dtb-y for unittest instead of overlay-y.
- Rename the commented dtb filess in unittest Makefile as .dtbo.
- Improved Makefile code (I am learning a lot every day :)

V6->V7:
- Dropped the first 4 patches, already merged.
- Patch 1/3 is new, suggested by Rob and slightly modified by me.
- Adapt Patch 3/3 to the new rule and name the overlay dtbs as .dtbo.

--
Viresh

Rob Herring (1):
kbuild: Add generic rule to apply fdtoverlay

Viresh Kumar (4):
kbuild: Simplify builds with CONFIG_OF_ALL_DTBS
kbuild: Allow .dtso format for overlay source files
of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
of: unittest: Statically apply overlays using fdtoverlay

 drivers/of/unittest-data/Makefile             | 48 ++++++++++
 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        | 23 ++---
 .../of/unittest-data/testcases_common.dtsi    | 19 ++++
 .../of/unittest-data/tests-interrupts.dtsi    | 11 +--
 scripts/Makefile.lib                          | 40 ++++++--
 9 files changed, 218 insertions(+), 112 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

base-commit: a38fd8748464831584a19438cbb3082b5a2dab15

Signature · 1 line

2.25.0.rc1.19.g042ed3e048af

Viresh Kumar

author

2021-03-10 05:36 UTC #1 lore raw

PATCH v11 1/5 kbuild: Simplify builds with CONFIG_OF_ALL_DTBS

We update 'extra-y' based on CONFIG_OF_ALL_DTBS three times. It would be far more straight forward if we rather update dtb-y to include all .dtb files if CONFIG_OF_ALL_DTBS is enabled.

Acked-by Masahiro Yamada <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 scripts/Makefile.lib | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

scripts/Makefile.lib +2 -3

index eee59184de64..a2658242d956 100644
@@ -73,14 +73,13 @@ always-y += $(userprogs-always-y) $(userprogs-always-m)
 
 # DTB
 # If CONFIG_OF_ALL_DTBS is enabled, all DT blobs are built
+dtb-$(CONFIG_OF_ALL_DTBS)       += $(dtb-)
+
 always-y			+= $(dtb-y)
-always-$(CONFIG_OF_ALL_DTBS)	+= $(dtb-)
 
 ifneq ($(CHECK_DTBS),)
 always-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
 always-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y))
-always-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
-always-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-))
 endif
 
 # Add subdir path

Signature · 1 line

2.25.0.rc1.19.g042ed3e048af

Viresh Kumar

author

2021-03-10 05:36 UTC #2 lore raw

PATCH v11 2/5 kbuild: Add generic rule to apply fdtoverlay

In reply to Viresh Kumar

From: Rob Herring <[email protected]>

Add a generic rule to apply fdtoverlay in Makefile.lib, so every platform doesn't need to carry the complex rule. This also automatically adds "DTC_FLAGS_foo_base += -@" for all base files.

The platform's Makefile only needs to have this now:

foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo dtb-y := foo.dtb

We don't want to run schema checks on foo.dtb (as foo.dts doesn't exist) and the Makefile is updated accordingly.

Acked-by Masahiro Yamada <[email protected]>

Signed-off-by Rob Herring <[email protected]>

Co-developed-by Viresh Kumar <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 scripts/Makefile.lib | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

scripts/Makefile.lib +24 -2

index a2658242d956..bc045a54a34e 100644
@@ -75,11 +75,24 @@ always-y += $(userprogs-always-y) $(userprogs-always-m)
 # If CONFIG_OF_ALL_DTBS is enabled, all DT blobs are built
 dtb-$(CONFIG_OF_ALL_DTBS)       += $(dtb-)
 
+# List all dtbs to be generated by fdtoverlay
+overlay-y := $(foreach m,$(dtb-y), $(if $(strip $($(m:.dtb=-dtbs))),$(m),))
+
+# Generate symbols for the base files so overlays can be applied to them.
+$(foreach m,$(overlay-y), $(eval DTC_FLAGS_$(basename $(firstword $($(m:.dtb=-dtbs)))) += -@))
+
+# Add base dtb and overlay dtbo
+dtb-y += $(foreach m,$(overlay-y), $($(m:.dtb=-dtbs)))
+
 always-y			+= $(dtb-y)
 
 ifneq ($(CHECK_DTBS),)
-always-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
-always-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y))
+# Don't run schema checks for dtbs created by fdtoverlay as they don't
+# have corresponding dts files.
+dt-yaml-y := $(filter-out $(overlay-y),$(dtb-y))
+
+always-y += $(patsubst %.dtb,%.dt.yaml, $(dt-yaml-y))
+always-y += $(patsubst %.dtbo,%.dt.yaml, $(dt-yaml-y))
 endif
 
 # Add subdir path

@@ -337,6 +350,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
 $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
 	$(call if_changed_dep,dtc)
 
+overlay-y := $(addprefix $(obj)/, $(overlay-y))
+
+quiet_cmd_fdtoverlay = DTOVL   $@
+      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
+
+$(overlay-y): FORCE
+	$(call if_changed,fdtoverlay)
+$(call multi_depend, $(overlay-y), .dtb, -dtbs)
+
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
 # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile

Signature · 1 line

2.25.0.rc1.19.g042ed3e048af

Viresh Kumar

author

2021-03-10 05:36 UTC #3 lore raw

PATCH v11 3/5 kbuild: Allow .dtso format for overlay source files

In reply to Viresh Kumar

Since the overlays dtb files are now named as .dtbo, there is a lot of interest in similarly naming the overlay source dts files as .dtso.

This patch makes the necessary changes to allow .dtso format for overlay source files.

Note that we still support generating .dtbo files from .dts files. This is required for the source files present in drivers/of/unittest-data/, because they can't be renamed to .dtso as they are used for some runtime testing as well.

Reviewed-by Geert Uytterhoeven <[email protected]>

Tested-by Geert Uytterhoeven <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 scripts/Makefile.lib | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

scripts/Makefile.lib +8 -1

index bc045a54a34e..59e86f67f9e0 100644
@@ -339,7 +339,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)

@@ -347,9 +347,13 @@ 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)
 
+# Required for of unit-test files as they can't be renamed to .dtso
 $(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   $@

@@ -375,6 +379,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

Signature · 1 line

2.25.0.rc1.19.g042ed3e048af

Masahiro Yamada

2021-03-10 11:27 UTC #4 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 24 lines · Viresh Kumar

Since the overlays dtb files are now named as .dtbo, there is a lot of interest in similarly naming the overlay source dts files as .dtso.
This patch makes the necessary changes to allow .dtso format for overlay source files.
Note that we still support generating .dtbo files from .dts files. This is required for the source files present in drivers/of/unittest-data/, because they can't be renamed to .dtso as they are used for some runtime testing as well.
Reviewed-by Geert Uytterhoeven <[email protected]>
Tested-by Geert Uytterhoeven <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
---
 scripts/Makefile.lib | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
scripts/Makefile.lib +0 -0
index bc045a54a34e..59e86f67f9e0 100644

scripts/Makefile.lib $(obj)/%.dtb.S: $(obj)/%.dtb FORCE · line 342

Quoted 5 lines

scripts/Makefile.lib +1 -1

@@ -339,7 +339,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 \

Even without "-I dts",

   inform = guess_input_format(arg, "dts");

seems to fall back to "dts" anyway,
but I guess you wanted to make this explicit, correct?

I will drop the ugly -O.
https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/

I will queue it to linux-kbuild/fixes.

scripts/Makefile.lib cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; · line 350

Quoted 8 lines

scripts/Makefile.lib +1 -0

                $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
                -d $(depfile).dtc.tmp $(dtc-tmp) ; \
        cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)

@@ -347,9 +347,13 @@ 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)
 
+# Required for of unit-test files as they can't be renamed to .dtso

If you go with *.dtso, I think you will rename
all *.dts under the drivers/ directory.

What is blocking you from making this consistent?

Quoted 22 lines

scripts/Makefile.lib +6 -1

 $(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   $@

@@ -375,6 +379,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
--

2.25.0.rc1.19.g042ed3e048af

Signature · 2 lines

Best Regards
Masahiro Yamada

Masahiro Yamada

2021-03-10 11:31 UTC #5 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 36 lines · Masahiro Yamada

Quoted 24 lines · Viresh Kumar
Since the overlays dtb files are now named as .dtbo, there is a lot of interest in similarly naming the overlay source dts files as .dtso.
This patch makes the necessary changes to allow .dtso format for overlay source files.
Note that we still support generating .dtbo files from .dts files. This is required for the source files present in drivers/of/unittest-data/, because they can't be renamed to .dtso as they are used for some runtime testing as well.
Reviewed-by Geert Uytterhoeven <[email protected]>
Tested-by Geert Uytterhoeven <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
---
 scripts/Makefile.lib | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
scripts/Makefile.lib +0 -0
index bc045a54a34e..59e86f67f9e0 100644
scripts/Makefile.lib $(obj)/%.dtb.S: $(obj)/%.dtb FORCE · line 342
Quoted 5 lines
scripts/Makefile.lib +1 -1
@@ -339,7 +339,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 \
Even without "-I dts",
   inform = guess_input_format(arg, "dts");

Quoted 9 lines

seems to fall back to "dts" anyway,
but I guess you wanted to make this explicit, correct?
I will drop the ugly -O.
https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/
I will queue it to linux-kbuild/fixes.

BTW, is the attached patch good for DTC?

I do not know when '-O dtbo' is useful,
unless I am missing something.

Signature · 2 lines

Best Regards
Masahiro Yamada

From eeac710289a22397c6ae4e90a9334c4f7e178688 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <[email protected]>
Date: Wed, 10 Mar 2021 19:48:05 +0900
Subject: [PATCH] dtc: Remove -O dtbo support

This partially reverts 163f0469bf2e ("dtc: Allow overlays to have .dtbo extension").

I do not understand why we need to support "dtbo" as --out-format.

*.dtb and *.dtbo have the same format, "dtb".

Signed-off-by Masahiro Yamada <[email protected]>

---
 dtc.c | 2 --
 1 file changed, 2 deletions(-)

dtc.c +0 -2

index 838c5df..3962d3f 100644
@@ -359,8 +359,6 @@ int main(int argc, char *argv[])
 #endif
 	} else if (streq(outform, "dtb")) {
 		dt_to_blob(outf, dti, outversion);
-	} else if (streq(outform, "dtbo")) {
-		dt_to_blob(outf, dti, outversion);
 	} else if (streq(outform, "asm")) {
 		dt_to_asm(outf, dti, outversion);
 	} else if (streq(outform, "null")) {

Signature · 1 line

2.27.0

Viresh Kumar

author

2021-03-10 14:49 UTC #6 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 4 lines · Masahiro Yamada

BTW, is the attached patch good for DTC?
I do not know when '-O dtbo' is useful,
unless I am missing something.

It is useful if we are sending the -O option all the time (I have already given more details to your patch) as outform will not be NULL.

Signature · 1 line

viresh

Masahiro Yamada

2021-03-10 15:20 UTC #7 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 9 lines · Viresh Kumar

Quoted 4 lines · Masahiro Yamada
BTW, is the attached patch good for DTC?
I do not know when '-O dtbo' is useful,
unless I am missing something.
It is useful if we are sending the -O option all the time (I have already given more details to your patch) as outform will not be NULL.

"-O dtbo" was useful to make your buggy patch work.

That is not justification.

.yaml   ->  -O yaml
.dtb    ->  -O dtb
.dtbo   ->  -O dtb

is the correct if you want to give -O explicitly.

Signature · 2 lines

Best Regards
Masahiro Yamada

Viresh Kumar

author

2021-03-12 03:53 UTC #8 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 8 lines · Masahiro Yamada

Quoted 7 lines · Viresh Kumar
Quoted 4 lines · Masahiro Yamada
BTW, is the attached patch good for DTC?
I do not know when '-O dtbo' is useful,
unless I am missing something.

Quoted 7 lines

Quoted 2 lines
It is useful if we are sending the -O option all the time (I have already given more details to your patch) as outform will not be NULL.
"-O dtbo" was useful to make your buggy patch work.
That is not justification.

I wasn't giving justification, but rather saying why it was required earlier. And I agree that it isn't required once we drop the -O parameter here.

Signature · 1 line

viresh

Viresh Kumar

author

2021-03-10 14:48 UTC #9 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

In reply to Masahiro Yamada

Quoted 14 lines · Masahiro Yamada

scripts/Makefile.lib $(obj)/%.dtb.S: $(obj)/%.dtb FORCE · line 342

Quoted 10 lines · Viresh Kumar

scripts/Makefile.lib +1 -1

index bc045a54a34e..59e86f67f9e0 100644
@@ -339,7 +339,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 \

Even without "-I dts",

Quoted 3 lines

   inform = guess_input_format(arg, "dts");
seems to fall back to "dts" anyway,

I missed this TBH.

Quoted 1 line

but I guess you wanted to make this explicit, correct?

That can be a reason now :)

Quoted 2 lines

I will drop the ugly -O.
https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/

But if we are going to depend on DTC to guess it right, then we shouldn't add -I at all..

Quoted 11 lines

I will queue it to linux-kbuild/fixes.

Quoted 7 lines

scripts/Makefile.lib +0 -0

                $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
                -d $(depfile).dtc.tmp $(dtc-tmp) ; \
        cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)

@@ -347,9 +347,13 @@ 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)

Quoted 6 lines

scripts/Makefile.lib cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; · line 350
Quoted 1 line
scripts/Makefile.lib +1 -0
+# Required for of unit-test files as they can't be renamed to .dtso
If you go with *.dtso, I think you will rename
all *.dts under the drivers/ directory.
What is blocking you from making this consistent?

The unit-test dts files are designed differently (we have had lots of discussion between Frank and David on that) and they aren't purely overlay or base files. They are designed to do some tricky testing and renaming them to .dtso won't be right, we are just reusing them to do static (build time) testing as well.

I think it would be better if we can drop the existing %.dtbo rule here (i.e. dtbo from .dts) and do some magic in unit-test's Makefile, so it is localised at least instead of it here.

Any ideas for that ?

Signature · 1 line

viresh

Masahiro Yamada

2021-03-10 15:17 UTC #10 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 48 lines · Viresh Kumar

Quoted 14 lines · Masahiro Yamada
scripts/Makefile.lib $(obj)/%.dtb.S: $(obj)/%.dtb FORCE · line 342
Quoted 10 lines · Viresh Kumar
scripts/Makefile.lib +1 -1
index bc045a54a34e..59e86f67f9e0 100644
@@ -339,7 +339,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 \
Even without "-I dts",
Quoted 3 lines
   inform = guess_input_format(arg, "dts");
seems to fall back to "dts" anyway,
I missed this TBH.
Quoted 1 line
but I guess you wanted to make this explicit, correct?
That can be a reason now :)
Quoted 2 lines
I will drop the ugly -O.
https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/
But if we are going to depend on DTC to guess it right, then we shouldn't add -I at all..
Quoted 16 lines
I will queue it to linux-kbuild/fixes.
scripts/Makefile.lib cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; · line 350
Quoted 8 lines
scripts/Makefile.lib +1 -0
                $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
                -d $(depfile).dtc.tmp $(dtc-tmp) ; \
        cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
@@ -347,9 +347,13 @@ 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)
 
+# Required for of unit-test files as they can't be renamed to .dtso
If you go with *.dtso, I think you will rename
all *.dts under the drivers/ directory.

Quoted 7 lines

Quoted 1 line
What is blocking you from making this consistent?
The unit-test dts files are designed differently (we have had lots of discussion between Frank and David on that) and they aren't purely overlay or base files. They are designed to do some tricky testing and renaming them to .dtso won't be right, we are just reusing them to do static (build time) testing as well.

I still do not understand.

If they are not overlay files, why
do you need to have them suffixed with .dtbo?

".dts -> .dtb" should be enough.

Why do you need to do ".dts -> .dtbo" ?

Quoted 5 lines

I think it would be better if we can drop the existing %.dtbo rule here (i.e. dtbo from .dts) and do some magic in unit-test's Makefile, so it is localised at least instead of it here.
Any ideas for that ?

I do not know.

My impression is you are doing something fishy.

Signature · 2 lines

Best Regards
Masahiro Yamada

Frank Rowand

2021-03-10 22:43 UTC #11 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 74 lines · Masahiro Yamada

Quoted 48 lines · Viresh Kumar
Quoted 14 lines · Masahiro Yamada
scripts/Makefile.lib $(obj)/%.dtb.S: $(obj)/%.dtb FORCE · line 342
Quoted 10 lines · Viresh Kumar
scripts/Makefile.lib +1 -1
index bc045a54a34e..59e86f67f9e0 100644
@@ -339,7 +339,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 \
Even without "-I dts",
Quoted 3 lines
   inform = guess_input_format(arg, "dts");
seems to fall back to "dts" anyway,
I missed this TBH.
Quoted 1 line
but I guess you wanted to make this explicit, correct?
That can be a reason now :)
Quoted 2 lines
I will drop the ugly -O.
https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/
But if we are going to depend on DTC to guess it right, then we shouldn't add -I at all..
Quoted 16 lines
I will queue it to linux-kbuild/fixes.
scripts/Makefile.lib cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; · line 350
Quoted 8 lines
scripts/Makefile.lib +1 -0
                $(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
                -d $(depfile).dtc.tmp $(dtc-tmp) ; \
        cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
@@ -347,9 +347,13 @@ 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)
 
+# Required for of unit-test files as they can't be renamed to .dtso
If you go with *.dtso, I think you will rename
all *.dts under the drivers/ directory.
Quoted 7 lines
Quoted 1 line
What is blocking you from making this consistent?
The unit-test dts files are designed differently (we have had lots of discussion between Frank and David on that) and they aren't purely overlay or base files. They are designed to do some tricky testing and renaming them to .dtso won't be right, we are just reusing them to do static (build time) testing as well.
I still do not understand.
If they are not overlay files, why
do you need to have them suffixed with .dtbo?
".dts -> .dtb" should be enough.
Why do you need to do ".dts -> .dtbo" ?
Quoted 4 lines
I think it would be better if we can drop the existing %.dtbo rule here (i.e. dtbo from .dts) and do some magic in unit-test's Makefile, so it is localised at least instead of it here.

Quoted 7 lines

Quoted 1 line
Any ideas for that ?
I do not know.
My impression is you are doing something fishy.

That is accurate. Devicetree unittest plays some tricks to enable testing to occur. These tricks will never be used anywhere else in the kernel.

-Frank

Viresh Kumar

author

2021-03-12 04:48 UTC #12 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

In reply to Masahiro Yamada

Quoted 8 lines · Masahiro Yamada

Even without "-I dts",
   inform = guess_input_format(arg, "dts");
seems to fall back to "dts" anyway,
but I guess you wanted to make this explicit, correct?

Quoted 6 lines

Quoted 1 line
+# Required for of unit-test files as they can't be renamed to .dtso
If you go with *.dtso, I think you will rename
all *.dts under the drivers/ directory.
What is blocking you from making this consistent?

What about this patch instead ? This localizes the dts->dtbo hack to unitest's Makefile at least.

drivers/of/unittest-data/Makefile +4 -0

index a5d2d9254b2c..9f3426ec3fab 100644
@@ -86,3 +86,7 @@ static_test_1-dtbs := static_base_1.dtb $(apply_static_overlay_1)
 static_test_2-dtbs := static_base_2.dtb $(apply_static_overlay_2)
 
 dtb-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb
+
+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)

scripts/Makefile.lib +4 -1

index bc045a54a34e..77a9be055e51 100644
@@ -347,7 +347,7 @@ 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
+$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
        $(call if_changed_dep,dtc)
 
 overlay-y := $(addprefix $(obj)/, $(overlay-y))

@@ -375,6 +375,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

Signature · 1 line

viresh

Frank Rowand

2021-03-12 07:04 UTC #13 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Hi Viresh,

Quoted 11 lines · Viresh Kumar

Quoted 10 lines · Masahiro Yamada
Even without "-I dts",
   inform = guess_input_format(arg, "dts");
seems to fall back to "dts" anyway,
but I guess you wanted to make this explicit, correct?
Quoted 1 line
+# Required for of unit-test files as they can't be renamed to .dtso

Quoted 7 lines

Quoted 4 lines
If you go with *.dtso, I think you will rename
all *.dts under the drivers/ directory.
What is blocking you from making this consistent?
What about this patch instead ? This localizes the dts->dtbo hack to unitest's Makefile at least.

It is late here, so I am not going to take the time to actually try what I am going to suggest. I apologize in advance if I send you off on a wild goose chase.

Would it work to create a .dtso file for each of the unittest overlay .dts files, where the .dtso would simply #include the .dts file. Then the corresponding .dtbo files could be added to the obj-$(CONFIG_OF_OVERLAY) list.

I would like to avoid having the unitest-data/Makefile have different rules to build objects because then the normal build rule is not being tested.

-Frank

Quoted 37 lines

drivers/of/unittest-data/Makefile +4 -0

index a5d2d9254b2c..9f3426ec3fab 100644
@@ -86,3 +86,7 @@ static_test_1-dtbs := static_base_1.dtb $(apply_static_overlay_1)
 static_test_2-dtbs := static_base_2.dtb $(apply_static_overlay_2)
 
 dtb-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb
+
+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)

scripts/Makefile.lib +4 -1

index bc045a54a34e..77a9be055e51 100644
@@ -347,7 +347,7 @@ 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
+$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
        $(call if_changed_dep,dtc)
 
 overlay-y := $(addprefix $(obj)/, $(overlay-y))

@@ -375,6 +375,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

Frank Rowand

2021-03-12 07:10 UTC #14 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 22 lines · Frank Rowand

Hi Viresh,
Quoted 11 lines · Viresh Kumar
Quoted 10 lines · Masahiro Yamada
Even without "-I dts",
   inform = guess_input_format(arg, "dts");
seems to fall back to "dts" anyway,
but I guess you wanted to make this explicit, correct?
Quoted 1 line
+# Required for of unit-test files as they can't be renamed to .dtso
Quoted 7 lines
Quoted 4 lines
If you go with *.dtso, I think you will rename
all *.dts under the drivers/ directory.
What is blocking you from making this consistent?
What about this patch instead ? This localizes the dts->dtbo hack to unitest's Makefile at least.

Quoted 7 lines

It is late here, so I am not going to take the time to actually try what I am going to suggest. I apologize in advance if I send you off on a wild goose chase.
Would it work to create a .dtso file for each of the unittest overlay .dts files, where the .dtso would simply #include the .dts file. Then the corresponding .dtbo files could be added to the obj-$(CONFIG_OF_OVERLAY) list.

I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.

If it works, I am fine with either approach.

-Frank

Quoted 44 lines

I would like to avoid having the unitest-data/Makefile have different rules to build objects because then the normal build rule is not being tested.

-Frank

Quoted 37 lines

drivers/of/unittest-data/Makefile +4 -0

index a5d2d9254b2c..9f3426ec3fab 100644
@@ -86,3 +86,7 @@ static_test_1-dtbs := static_base_1.dtb $(apply_static_overlay_1)
 static_test_2-dtbs := static_base_2.dtb $(apply_static_overlay_2)
 
 dtb-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb
+
+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)

scripts/Makefile.lib +4 -1

index bc045a54a34e..77a9be055e51 100644
@@ -347,7 +347,7 @@ 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
+$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
        $(call if_changed_dep,dtc)
 
 overlay-y := $(addprefix $(obj)/, $(overlay-y))

@@ -375,6 +375,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

Viresh Kumar

author

2021-03-12 07:14 UTC #15 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 6 lines · Frank Rowand

I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.

What about creating links instead then ?

Signature · 1 line

viresh

Frank Rowand

2021-03-13 05:11 UTC #16 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 10 lines · Viresh Kumar

Quoted 6 lines · Frank Rowand
I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.
What about creating links instead then ?

I don't really like the idea of using links here.

Maybe it is best to make the changes needed to allow the unittest overlays to be .dtso instead of .dts.

Off the top of my head:

scripts/Makefile.lib:
The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the overlay data in section .dtb.init.rodata, with a label pointing to the beginning of the overlay __dtb_XXX_begin and a label pointing to the end of the overlay __dtb_XXX_end, for the overlay named XXX. I _think_ that you could simply add a corresponding rule for %.dtbo.S using a new command cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels __dtbo_XXX_begin and __dtbo_XXX_end).

drivers/of/unittest.o:
would need to have the #define of OVERLAY_INFO() changed to reflect the changed label names (use __dtbo_##overlayname##begin and __dtb_##overlay_name##_end).

  drivers/of/unittest-data/Makefile:
     In obj-$(CONFIG_OF_OVERLAY) change the *.dtb.o names to *.dtbo.o

I'm not sure how the DTC_FLAGS_... += -@ differentiates between .dts / .dtb and .dtso / .dtbo That is worth looking at.

-Frank

Frank Rowand

2021-03-15 01:16 UTC #17 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Hi VIresh,

Quoted 19 lines · Frank Rowand

Quoted 10 lines · Viresh Kumar
Quoted 6 lines · Frank Rowand
I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.
What about creating links instead then ?
I don't really like the idea of using links here.
Maybe it is best to make the changes needed to allow the unittest overlays to be .dtso instead of .dts.
Off the top of my head:

Quoted 9 lines

scripts/Makefile.lib:
The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the overlay data in section .dtb.init.rodata, with a label pointing to the beginning of the overlay __dtb_XXX_begin and a label pointing to the end of the overlay __dtb_XXX_end, for the overlay named XXX. I _think_ that you could simply add a corresponding rule for %.dtbo.S using a new command cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels __dtbo_XXX_begin and __dtbo_XXX_end).

If you do the above, please put it in drivers/of/unittest-data/Makefile instead of scripts/Makefile.lib because it is unittest.c specific and not meant to be anywhere else in the kernel.

-Frank

Quoted 14 lines

drivers/of/unittest.o:
would need to have the #define of OVERLAY_INFO() changed to reflect the changed label names (use __dtbo_##overlayname##begin and __dtb_##overlay_name##_end).
  drivers/of/unittest-data/Makefile:
     In obj-$(CONFIG_OF_OVERLAY) change the *.dtb.o names to *.dtbo.o
I'm not sure how the DTC_FLAGS_... += -@ differentiates between .dts / .dtb and .dtso / .dtbo That is worth looking at.
-Frank

Viresh Kumar

author

2021-03-15 06:41 UTC #18 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 30 lines · Frank Rowand

Quoted 19 lines · Frank Rowand
Quoted 10 lines · Viresh Kumar
Quoted 6 lines · Frank Rowand
I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.
What about creating links instead then ?
I don't really like the idea of using links here.
Maybe it is best to make the changes needed to allow the unittest overlays to be .dtso instead of .dts.
Off the top of my head:
Quoted 9 lines
scripts/Makefile.lib:
The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the overlay data in section .dtb.init.rodata, with a label pointing to the beginning of the overlay __dtb_XXX_begin and a label pointing to the end of the overlay __dtb_XXX_end, for the overlay named XXX. I _think_ that you could simply add a corresponding rule for %.dtbo.S using a new command cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels __dtbo_XXX_begin and __dtbo_XXX_end).

Quoted 3 lines

If you do the above, please put it in drivers/of/unittest-data/Makefile instead of scripts/Makefile.lib because it is unittest.c specific and not meant to be anywhere else in the kernel.

What about doing this then in unittest's Makefile instead (which I already suggested earlier), that will make everything work just fine without any other changes ?

+# Required for of unittest files as they can't be renamed to .dtso +$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+ $(call if_changed_dep,dtc)

Signature · 1 line

viresh

Masahiro Yamada

2021-03-15 17:56 UTC #19 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 45 lines · Viresh Kumar

Quoted 30 lines · Frank Rowand
Quoted 19 lines · Frank Rowand
Quoted 10 lines · Viresh Kumar
Quoted 6 lines · Frank Rowand
I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.
What about creating links instead then ?
I don't really like the idea of using links here.
Maybe it is best to make the changes needed to allow the unittest overlays to be .dtso instead of .dts.
Off the top of my head:
Quoted 9 lines
scripts/Makefile.lib:
The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the overlay data in section .dtb.init.rodata, with a label pointing to the beginning of the overlay __dtb_XXX_begin and a label pointing to the end of the overlay __dtb_XXX_end, for the overlay named XXX. I _think_ that you could simply add a corresponding rule for %.dtbo.S using a new command cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels __dtbo_XXX_begin and __dtbo_XXX_end).
Quoted 3 lines
If you do the above, please put it in drivers/of/unittest-data/Makefile instead of scripts/Makefile.lib because it is unittest.c specific and not meant to be anywhere else in the kernel.
What about doing this then in unittest's Makefile instead (which I already suggested earlier), that will make everything work just fine without any other changes ?
(quoted diff) +3 -1
+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)
 
--

Quoted 1 line

viresh

If those rules are only needed by drivers/of/unittest-data/Makefile, they should not be located in scripts/Makefile.lib.

But how can we fix drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a779*.dts if these are doing bad things.
They seem to be overlay files even though the file name suffix is .dts

$ find drivers -name '*.dts'
drivers/staging/pi433/Documentation/devicetree/pi433-overlay.dts
drivers/staging/mt7621-dts/gbpc2.dts
drivers/staging/mt7621-dts/gbpc1.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
drivers/of/unittest-data/overlay_1.dts
drivers/of/unittest-data/testcases.dts
drivers/of/unittest-data/overlay_bad_add_dup_node.dts
drivers/of/unittest-data/overlay_bad_symbol.dts
drivers/of/unittest-data/overlay_0.dts
drivers/of/unittest-data/overlay_11.dts
drivers/of/unittest-data/overlay_gpio_03.dts
drivers/of/unittest-data/overlay_gpio_04a.dts
drivers/of/unittest-data/overlay_gpio_04b.dts
drivers/of/unittest-data/overlay_5.dts
drivers/of/unittest-data/overlay_bad_add_dup_prop.dts
drivers/of/unittest-data/overlay_gpio_01.dts
drivers/of/unittest-data/overlay_10.dts
drivers/of/unittest-data/overlay_7.dts
drivers/of/unittest-data/overlay_bad_phandle.dts
drivers/of/unittest-data/overlay_3.dts
drivers/of/unittest-data/overlay_6.dts
drivers/of/unittest-data/overlay_8.dts
drivers/of/unittest-data/overlay_12.dts
drivers/of/unittest-data/overlay_gpio_02a.dts
drivers/of/unittest-data/overlay_gpio_02b.dts
drivers/of/unittest-data/overlay_4.dts
drivers/of/unittest-data/overlay.dts
drivers/of/unittest-data/overlay_9.dts
drivers/of/unittest-data/overlay_2.dts
drivers/of/unittest-data/overlay_15.dts
drivers/of/unittest-data/overlay_base.dts
drivers/of/unittest-data/overlay_13.dts

Signature · 2 lines

Best Regards
Masahiro Yamada

Laurent Pinchart

2021-03-15 22:13 UTC #20 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Hi Yamada-san,

Quoted 45 lines · Masahiro Yamada

Quoted 40 lines · Viresh Kumar
Quoted 30 lines · Frank Rowand
Quoted 19 lines · Frank Rowand
Quoted 10 lines · Viresh Kumar
Quoted 6 lines · Frank Rowand
I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.
What about creating links instead then ?
I don't really like the idea of using links here.
Maybe it is best to make the changes needed to allow the unittest overlays to be .dtso instead of .dts.
Off the top of my head:
Quoted 9 lines
scripts/Makefile.lib:
The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the overlay data in section .dtb.init.rodata, with a label pointing to the beginning of the overlay __dtb_XXX_begin and a label pointing to the end of the overlay __dtb_XXX_end, for the overlay named XXX. I _think_ that you could simply add a corresponding rule for %.dtbo.S using a new command cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels __dtbo_XXX_begin and __dtbo_XXX_end).
Quoted 3 lines
If you do the above, please put it in drivers/of/unittest-data/Makefile instead of scripts/Makefile.lib because it is unittest.c specific and not meant to be anywhere else in the kernel.
What about doing this then in unittest's Makefile instead (which I already suggested earlier), that will make everything work just fine without any other changes ?
Review
Quoted 3 lines
(quoted diff) +3 -0
+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)

Quoted 6 lines

If those rules are only needed by drivers/of/unittest-data/Makefile, they should not be located in scripts/Makefile.lib.
But how can we fix drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a779*.dts if these are doing bad things.
They seem to be overlay files even though the file name suffix is .dts

That is correct, they are overlays. I have no issue with those files being renamed to .dtso if that can help (but I haven't checked if that would have any adverse effect on the R-Car DU driver).

These files are there to ensure backward compatibility with older DT bindings. The change was made 3 years ago and I wouldn't object to dropping this completely, but I understand I may not be the most cautious person when it comes to ensuring DT backward compatibility :-)

Quoted 37 lines

$ find drivers -name '*.dts'
drivers/staging/pi433/Documentation/devicetree/pi433-overlay.dts
drivers/staging/mt7621-dts/gbpc2.dts
drivers/staging/mt7621-dts/gbpc1.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
drivers/of/unittest-data/overlay_1.dts
drivers/of/unittest-data/testcases.dts
drivers/of/unittest-data/overlay_bad_add_dup_node.dts
drivers/of/unittest-data/overlay_bad_symbol.dts
drivers/of/unittest-data/overlay_0.dts
drivers/of/unittest-data/overlay_11.dts
drivers/of/unittest-data/overlay_gpio_03.dts
drivers/of/unittest-data/overlay_gpio_04a.dts
drivers/of/unittest-data/overlay_gpio_04b.dts
drivers/of/unittest-data/overlay_5.dts
drivers/of/unittest-data/overlay_bad_add_dup_prop.dts
drivers/of/unittest-data/overlay_gpio_01.dts
drivers/of/unittest-data/overlay_10.dts
drivers/of/unittest-data/overlay_7.dts
drivers/of/unittest-data/overlay_bad_phandle.dts
drivers/of/unittest-data/overlay_3.dts
drivers/of/unittest-data/overlay_6.dts
drivers/of/unittest-data/overlay_8.dts
drivers/of/unittest-data/overlay_12.dts
drivers/of/unittest-data/overlay_gpio_02a.dts
drivers/of/unittest-data/overlay_gpio_02b.dts
drivers/of/unittest-data/overlay_4.dts
drivers/of/unittest-data/overlay.dts
drivers/of/unittest-data/overlay_9.dts
drivers/of/unittest-data/overlay_2.dts
drivers/of/unittest-data/overlay_15.dts
drivers/of/unittest-data/overlay_base.dts
drivers/of/unittest-data/overlay_13.dts

Signature · 3 lines

Regards,

Laurent Pinchart

Frank Rowand

2021-03-16 05:40 UTC #21 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 51 lines · Laurent Pinchart

Hi Yamada-san,
Quoted 48 lines · Masahiro Yamada
Quoted 40 lines · Viresh Kumar
Quoted 30 lines · Frank Rowand
Quoted 19 lines · Frank Rowand
Quoted 10 lines · Viresh Kumar
Quoted 6 lines · Frank Rowand
I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.
What about creating links instead then ?
I don't really like the idea of using links here.
Maybe it is best to make the changes needed to allow the unittest overlays to be .dtso instead of .dts.
Off the top of my head:
Quoted 9 lines
scripts/Makefile.lib:
The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the overlay data in section .dtb.init.rodata, with a label pointing to the beginning of the overlay __dtb_XXX_begin and a label pointing to the end of the overlay __dtb_XXX_end, for the overlay named XXX. I _think_ that you could simply add a corresponding rule for %.dtbo.S using a new command cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels __dtbo_XXX_begin and __dtbo_XXX_end).
Quoted 3 lines
If you do the above, please put it in drivers/of/unittest-data/Makefile instead of scripts/Makefile.lib because it is unittest.c specific and not meant to be anywhere else in the kernel.
What about doing this then in unittest's Makefile instead (which I already suggested earlier), that will make everything work just fine without any other changes ?
Review
Quoted 3 lines
(quoted diff) +3 -0
+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)
If those rules are only needed by drivers/of/unittest-data/Makefile, they should not be located in scripts/Makefile.lib.

Quoted 7 lines

Quoted 3 lines
But how can we fix drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a779*.dts if these are doing bad things.
They seem to be overlay files even though the file name suffix is .dts
That is correct, they are overlays. I have no issue with those files being renamed to .dtso if that can help (but I haven't checked if that would have any adverse effect on the R-Car DU driver).

As Laurent replied, yes these are overlays. They were grandfathered in as a deprecated use of overlays.

Quoted 5 lines

These files are there to ensure backward compatibility with older DT bindings. The change was made 3 years ago and I wouldn't object to dropping this completely, but I understand I may not be the most cautious person when it comes to ensuring DT backward compatibility :-)

My memory is that the goal was to eventually remove these overlays at some point in the future. If everyone agrees that today is the proper time, it would be helpful to go ahead and remove these .dts files and the code that uses them.

-Frank

Quoted 39 lines

Quoted 38 lines

$ find drivers -name '*.dts'
drivers/staging/pi433/Documentation/devicetree/pi433-overlay.dts
drivers/staging/mt7621-dts/gbpc2.dts
drivers/staging/mt7621-dts/gbpc1.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
drivers/of/unittest-data/overlay_1.dts
drivers/of/unittest-data/testcases.dts
drivers/of/unittest-data/overlay_bad_add_dup_node.dts
drivers/of/unittest-data/overlay_bad_symbol.dts
drivers/of/unittest-data/overlay_0.dts
drivers/of/unittest-data/overlay_11.dts
drivers/of/unittest-data/overlay_gpio_03.dts
drivers/of/unittest-data/overlay_gpio_04a.dts
drivers/of/unittest-data/overlay_gpio_04b.dts
drivers/of/unittest-data/overlay_5.dts
drivers/of/unittest-data/overlay_bad_add_dup_prop.dts
drivers/of/unittest-data/overlay_gpio_01.dts
drivers/of/unittest-data/overlay_10.dts
drivers/of/unittest-data/overlay_7.dts
drivers/of/unittest-data/overlay_bad_phandle.dts
drivers/of/unittest-data/overlay_3.dts
drivers/of/unittest-data/overlay_6.dts
drivers/of/unittest-data/overlay_8.dts
drivers/of/unittest-data/overlay_12.dts
drivers/of/unittest-data/overlay_gpio_02a.dts
drivers/of/unittest-data/overlay_gpio_02b.dts
drivers/of/unittest-data/overlay_4.dts
drivers/of/unittest-data/overlay.dts
drivers/of/unittest-data/overlay_9.dts
drivers/of/unittest-data/overlay_2.dts
drivers/of/unittest-data/overlay_15.dts
drivers/of/unittest-data/overlay_base.dts
drivers/of/unittest-data/overlay_13.dts

Geert Uytterhoeven

2021-03-16 08:02 UTC #22 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Hi Frank,

Quoted 18 lines · Frank Rowand

Quoted 8 lines · Laurent Pinchart
Quoted 3 lines · Masahiro Yamada
But how can we fix drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a779*.dts if these are doing bad things.
They seem to be overlay files even though the file name suffix is .dts
That is correct, they are overlays. I have no issue with those files being renamed to .dtso if that can help (but I haven't checked if that would have any adverse effect on the R-Car DU driver).
As Laurent replied, yes these are overlays. They were grandfathered in as a deprecated use of overlays.
Quoted 4 lines
These files are there to ensure backward compatibility with older DT bindings. The change was made 3 years ago and I wouldn't object to dropping this completely, but I understand I may not be the most cautious person when it comes to ensuring DT backward compatibility :-)

Quoted 4 lines

My memory is that the goal was to eventually remove these overlays at some point in the future. If everyone agrees that today is the proper time, it would be helpful to go ahead and remove these .dts files and the code that uses them.

Given [1][2][3] were merged in v4.17, and [4] was merged in v4.20, and all were backported to the old v4.14-based R-Car BSP v3.8.0, I think it's safe to assume all users have the DTS updates, so the backward compatibility mode can be removed?

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts

[1] 15a1ff30d8f9bd83 ("ARM: dts: r8a7790: Convert to new LVDS DT bindings")
[2] e5c3f4707f3956a2 ("ARM: dts: r8a7791: Convert to new LVDS DT bindings")
[3] edb0c3affe5214a2 ("ARM: dts: r8a7793: Convert to new LVDS DT bindings")
[4] 58e8ed2ee9abe718 ("arm64: dts: renesas: Convert to new LVDS DT bindings")

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

Masahiro Yamada

2021-04-09 06:04 UTC #23 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 38 lines · Geert Uytterhoeven

Hi Frank,
Quoted 18 lines · Frank Rowand
Quoted 8 lines · Laurent Pinchart
Quoted 3 lines · Masahiro Yamada
But how can we fix drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a779*.dts if these are doing bad things.
They seem to be overlay files even though the file name suffix is .dts
That is correct, they are overlays. I have no issue with those files being renamed to .dtso if that can help (but I haven't checked if that would have any adverse effect on the R-Car DU driver).
As Laurent replied, yes these are overlays. They were grandfathered in as a deprecated use of overlays.
Quoted 4 lines
These files are there to ensure backward compatibility with older DT bindings. The change was made 3 years ago and I wouldn't object to dropping this completely, but I understand I may not be the most cautious person when it comes to ensuring DT backward compatibility :-)
Quoted 4 lines
My memory is that the goal was to eventually remove these overlays at some point in the future. If everyone agrees that today is the proper time, it would be helpful to go ahead and remove these .dts files and the code that uses them.
Given [1][2][3] were merged in v4.17, and [4] was merged in v4.20, and all were backported to the old v4.14-based R-Car BSP v3.8.0, I think it's safe to assume all users have the DTS updates, so the backward compatibility mode can be removed?
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts

Quoted 4 lines

[1] 15a1ff30d8f9bd83 ("ARM: dts: r8a7790: Convert to new LVDS DT bindings")
[2] e5c3f4707f3956a2 ("ARM: dts: r8a7791: Convert to new LVDS DT bindings")
[3] edb0c3affe5214a2 ("ARM: dts: r8a7793: Convert to new LVDS DT bindings")
[4] 58e8ed2ee9abe718 ("arm64: dts: renesas: Convert to new LVDS DT bindings")

Can we remove all of drivers/gpu/drm/rcar-du/*.dts ?

I see some more under drivers/staging/.

masahiro@grover:~/ref/linux-next$ find  drivers  -name  '*.dts'  |
grep -v drivers/of/unittest-data
drivers/staging/pi433/Documentation/devicetree/pi433-overlay.dts
drivers/staging/mt7621-dts/gbpc2.dts
drivers/staging/mt7621-dts/gbpc1.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts

Best Regards

Masahiro Yamada

Viresh Kumar

author

2021-03-16 05:28 UTC #24 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

In reply to Masahiro Yamada

Quoted 6 lines · Masahiro Yamada

Quoted 5 lines · Viresh Kumar
On 14-03-21, 20:16, Frank Rowand wrote:
What about doing this then in unittest's Makefile instead (which I already suggested earlier), that will make everything work just fine without any other changes ?

Quoted 6 lines

Review
Quoted 3 lines
(quoted diff) +3 -0
+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)
If those rules are only needed by drivers/of/unittest-data/Makefile, they should not be located in scripts/Makefile.lib.

Right, this is exactly what I suggested.

Quoted 4 lines

But how can we fix drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a779*.dts if these are doing bad things.
They seem to be overlay files even though the file name suffix is .dts

Quoted 9 lines

$ find drivers -name '*.dts'
drivers/staging/pi433/Documentation/devicetree/pi433-overlay.dts
drivers/staging/mt7621-dts/gbpc2.dts
drivers/staging/mt7621-dts/gbpc1.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts

For all the above files, even if they are really overlay files, we won't use fdtoverlay tool to apply them to some base dtb and so if we leave them as is, i.e. .dts->.dtb, it won't break anything.

The problem only happens if someone wants to generate .dtbo for them instead and then they should be named .dtso as we won't allow .dts -> .dtbo conversion there.

Signature · 1 line

viresh

Frank Rowand

2021-03-16 05:37 UTC #25 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

In reply to Viresh Kumar

Quoted 40 lines · Viresh Kumar

Quoted 30 lines · Frank Rowand
Quoted 19 lines · Frank Rowand
Quoted 10 lines · Viresh Kumar
Quoted 6 lines · Frank Rowand
I suggested having the .dtso files include the .dts file because that is a relatively small and easy change to test. What would probably make more sense is the rename the existing overlay .dts files to be .dtso files and then for each overlay .dtso file create a new .dts file that #includes the corresponding .dtso file. This is more work and churn, but easier to document that the .dts files are a hack that is needed so that the corresponding .dtb.S files will be generated.
What about creating links instead then ?
I don't really like the idea of using links here.
Maybe it is best to make the changes needed to allow the unittest overlays to be .dtso instead of .dts.
Off the top of my head:
Quoted 9 lines
scripts/Makefile.lib:
The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the overlay data in section .dtb.init.rodata, with a label pointing to the beginning of the overlay __dtb_XXX_begin and a label pointing to the end of the overlay __dtb_XXX_end, for the overlay named XXX. I _think_ that you could simply add a corresponding rule for %.dtbo.S using a new command cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels __dtbo_XXX_begin and __dtbo_XXX_end).
Quoted 3 lines
If you do the above, please put it in drivers/of/unittest-data/Makefile instead of scripts/Makefile.lib because it is unittest.c specific and not meant to be anywhere else in the kernel.
What about doing this then in unittest's Makefile instead (which I already suggested earlier), that will make everything work just fine without any other changes ?

Review

Quoted 3 lines

(quoted diff) +3 -0

+# Required for of unittest files as they can't be renamed to .dtso
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+       $(call if_changed_dep,dtc)

I should have looked at patch 3/5 more carefully instead of counting on Masahiro to check it out and simply build testing.

Patch 3/5 does not seem correct. I'll look over all the makefile related changes that have been accepted (if any) and patch 3/5 later today (Tuesday).

-Frank

Viresh Kumar

author

2021-03-16 05:43 UTC #26 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 5 lines · Frank Rowand

I should have looked at patch 3/5 more carefully instead of counting on Masahiro to check it out and simply build testing.
Patch 3/5 does not seem correct. I'll look over all the makefile related changes that have been accepted (if any) and patch 3/5 later today (Tuesday).

What is already merged by Rob is what everyone reviewed and it wasn't related to this .dtso/.dtbo discussion we are having. I will resend a new patchset with the stuff left for merging (which will involve the thing we are discussing here), so we can get a clear picture of it.

Signature · 1 line

viresh

Frank Rowand

2021-03-24 07:35 UTC #27 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Quoted 4 lines · Viresh Kumar

Quoted 3 lines · Frank Rowand
I should have looked at patch 3/5 more carefully instead of counting on Masahiro to check it out and simply build testing.

Quoted 8 lines

Quoted 2 lines
Patch 3/5 does not seem correct. I'll look over all the makefile related changes that have been accepted (if any) and patch 3/5 later today (Tuesday).
What is already merged by Rob is what everyone reviewed and it wasn't related to this .dtso/.dtbo discussion we are having. I will resend a new patchset with the stuff left for merging (which will involve the thing we are discussing here), so we can get a clear picture of it.

Instead of doing what I suggested in my 16-03-21, 00:36 email (only partly quoted above), I have made most of the changes to unittest.c and drivers/of/unitest/unittest-data/Makefile to allow the unittest .dts files to be .dtso source files and .dtbo FDT files, and tested. One final step remaining in my work is to actually rename the *.dts files to *.dtso.

I hope to finish this later today (Wednesday) after getting some sleep.

-Frank

Frank Rowand

2021-03-24 22:46 UTC #28 lore raw

Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

Hi Viresh,

Quoted 21 lines · Frank Rowand

Quoted 4 lines · Viresh Kumar
Quoted 3 lines · Frank Rowand
I should have looked at patch 3/5 more carefully instead of counting on Masahiro to check it out and simply build testing.
Quoted 8 lines
Quoted 2 lines
Patch 3/5 does not seem correct. I'll look over all the makefile related changes that have been accepted (if any) and patch 3/5 later today (Tuesday).
What is already merged by Rob is what everyone reviewed and it wasn't related to this .dtso/.dtbo discussion we are having. I will resend a new patchset with the stuff left for merging (which will involve the thing we are discussing here), so we can get a clear picture of it.
Instead of doing what I suggested in my 16-03-21, 00:36 email (only partly quoted above), I have made most of the changes to unittest.c and drivers/of/unitest/unittest-data/Makefile to allow the unittest .dts files to be .dtso source files and .dtbo FDT files, and tested. One final step remaining in my work is to actually rename the *.dts files to *.dtso.

Quoted 2 lines

I hope to finish this later today (Wednesday) after getting some sleep.

I have submitted a patch that I _think_ removes the need for most of patch 3/5, _other_ than the yaml rule, which I would assume is still needed, though I did not examine it.

My patch is
https://lore.kernel.org/r/[email protected]

Can you do a new patch with just the yaml rule (if Rob accepts my patch).

-Frank

Viresh Kumar

author

2021-03-10 05:36 UTC #29 lore raw

PATCH v11 4/5 of: unittest: Create overlay_common.dtsi and testcases_common.dtsi

In reply to Viresh Kumar

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        | 23 ++---
 .../of/unittest-data/testcases_common.dtsi    | 19 ++++
 .../of/unittest-data/tests-interrupts.dtsi    | 11 +--
 5 files changed, 128 insertions(+), 106 deletions(-)
 create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
 create mode 100644 drivers/of/unittest-data/testcases_common.dtsi

drivers/of/unittest-data/overlay_base.dts +1 ~~-89~~

index 99ab9d12d00b..ab9014589c5d 100644
@@ -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"

drivers/of/unittest-data/overlay_common.dtsi new file +91 -0

91 lines not on this page · raw message

drivers/of/unittest-data/testcases.dts +12 ~~-11~~

index a85b5e1c381a..61cdd3d5fccb 100644
@@ -2,19 +2,20 @@
 /dts-v1/;
 /plugin/;
 
+#include "testcases_common.dtsi"
+
 / {
+	/*
+	 * 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"

drivers/of/unittest-data/testcases_common.dtsi new file +19 -0

new file mode 100644
index 000000000000..19292bbb4cbb
@@ -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"

drivers/of/unittest-data/tests-interrupts.dtsi +5 -6

index ec175e800725..9b60a549f502 100644
@@ -62,11 +62,10 @@ testcase-device1 {
 			interrupts = <1>;
 		};
 
-		testcase-device2 {
-			compatible = "testcase-device";
-			interrupt-parent = <&test_intc2>;
-			interrupts = <1>; /* invalid specifier - too short */
-		};
+		/*
+		 * 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.
+		 */
 	};
-
 };

Signature · 1 line

2.25.0.rc1.19.g042ed3e048af

Viresh Kumar

author

2021-03-10 05:36 UTC #30 lore raw

PATCH v11 5/5 of: unittest: Statically apply overlays using fdtoverlay

In reply to Viresh Kumar

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          | 48 ++++++++++++++++++++++
 drivers/of/unittest-data/static_base_1.dts |  4 ++
 drivers/of/unittest-data/static_base_2.dts |  4 ++
 3 files changed, 56 insertions(+)
 create mode 100644 drivers/of/unittest-data/static_base_1.dts
 create mode 100644 drivers/of/unittest-data/static_base_2.dts

drivers/of/unittest-data/Makefile +48 -0

index 009f4045c8e4..a5d2d9254b2c 100644
@@ -38,3 +38,51 @@ 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.dtbo \
+#			  overlay_bad_add_dup_prop.dtbo \
+#			  overlay_bad_phandle.dtbo \
+#			  overlay_bad_symbol.dtbo \
+
+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.dtbo
+
+static_test_1-dtbs := static_base_1.dtb $(apply_static_overlay_1)
+static_test_2-dtbs := static_base_2.dtb $(apply_static_overlay_2)
+
+dtb-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb

drivers/of/unittest-data/static_base_1.dts new file +4 -0

new file mode 100644
index 000000000000..10556cb3f01f
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "testcases_common.dtsi"

drivers/of/unittest-data/static_base_2.dts new file +4 -0

new file mode 100644
index 000000000000..b0ea9504d6f3
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "overlay_common.dtsi"

Signature · 1 line

2.25.0.rc1.19.g042ed3e048af

Frank Rowand

Reviewed-by Tested-by on the v11 series

2021-03-11 23:27 UTC #31 lore raw

In reply to Viresh Kumar

Quoted 61 lines · Viresh Kumar

Hi,
This patchset adds a generic rule for applying overlays using fdtoverlay tool and then updates unittests to get built statically using the same.
V10->V11:
- Update patch 4/5 to fix checkpatch warning on spaces and tabs.
- Added Acked-by from Masahiro for patch 2/5.
V9->V10:
- Add a new patch to allow .dtso files.
- Update 2/5 to be more efficient and also generate symbols for base files automatically.
- No need to add lines like DTC_FLAGS_foo_base += -@ in patch 5/5.
- Add Ack by Masahiro for 1/5.
V8->V9:
- Added some comment in patch 3/4 based on Frank's suggestions.
V7->V8:
- Patch 1 is new.
- Platforms need to use dtb-y += foo.dtb instead of overlay-y += foo.dtb.
- Use multi_depend instead of .SECONDEXPANSION.
- Use dtb-y for unittest instead of overlay-y.
- Rename the commented dtb filess in unittest Makefile as .dtbo.
- Improved Makefile code (I am learning a lot every day :)
V6->V7:
- Dropped the first 4 patches, already merged.
- Patch 1/3 is new, suggested by Rob and slightly modified by me.
- Adapt Patch 3/3 to the new rule and name the overlay dtbs as .dtbo.
--
Viresh
Rob Herring (1):
kbuild: Add generic rule to apply fdtoverlay
Viresh Kumar (4):
kbuild: Simplify builds with CONFIG_OF_ALL_DTBS
kbuild: Allow .dtso format for overlay source files
of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
of: unittest: Statically apply overlays using fdtoverlay
 drivers/of/unittest-data/Makefile             | 48 ++++++++++
 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        | 23 ++---
 .../of/unittest-data/testcases_common.dtsi    | 19 ++++
 .../of/unittest-data/tests-interrupts.dtsi    | 11 +--
 scripts/Makefile.lib                          | 40 ++++++--
 9 files changed, 218 insertions(+), 112 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

Quoted 2 lines

base-commit: a38fd8748464831584a19438cbb3082b5a2dab15

Does not apply to 5.12-rc2 because of a dependency on a patch to scripts/Makefile.lib. That patch has been merged by Linus somewhere between -rc2 and -rc3. I had a working version between -rc2 and -rc3 at commit e6f197677b2e that does have the required patch, so that is the version I used to test this series.

There is still confusion caused by the contortions that unittest goes through to mis-use base DTBs vs overlay DTBs, so _after_ this series is merged by Rob, I will poke around and see if I can change unittest so that it does not look like it is mis-using DTBs and overlay DTBs.

Reviewed-by Frank Rowand <[email protected]>

Tested-by Frank Rowand <[email protected]>

Viresh Kumar

author

2021-03-12 04:31 UTC #32 lore raw

Quoted 23 lines · Frank Rowand

Quoted 22 lines · Viresh Kumar

 drivers/of/unittest-data/Makefile             | 48 ++++++++++
 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        | 23 ++---
 .../of/unittest-data/testcases_common.dtsi    | 19 ++++
 .../of/unittest-data/tests-interrupts.dtsi    | 11 +--
 scripts/Makefile.lib                          | 40 ++++++--
 9 files changed, 218 insertions(+), 112 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

Quoted 4 lines

Quoted 2 lines
base-commit: a38fd8748464831584a19438cbb3082b5a2dab15
Does not apply to 5.12-rc2

I was based right over the 5.12-rc2 tag.

Quoted 3 lines

because of a dependency on a patch to scripts/Makefile.lib. That patch has been merged by Linus somewhere between -rc2 and -rc3.

git log --oneline v5.12-rc2..origin/master -- scripts/Makefile.lib

gives no results to me.

Quoted 2 lines

I had a working version
between -rc2 and -rc3 at commit e6f197677b2e

I have tried both Linus' tree and linux-next, and I don't see this commit.

Quoted 11 lines

that does have
the required patch, so that is the version I used to test this series.
There is still confusion caused by the contortions that unittest goes through to mis-use base DTBs vs overlay DTBs, so _after_ this series is merged by Rob, I will poke around and see if I can change unittest so that it does not look like it is mis-using DTBs and overlay DTBs.

Quoted 2 lines

Reviewed-by Frank Rowand <[email protected]>
Tested-by Frank Rowand <[email protected]>

Thanks.

Signature · 1 line

viresh

Frank Rowand

2021-03-12 06:47 UTC #33 lore raw

Quoted 24 lines · Viresh Kumar

Quoted 23 lines · Frank Rowand

Quoted 22 lines · Viresh Kumar

 drivers/of/unittest-data/Makefile             | 48 ++++++++++
 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        | 23 ++---
 .../of/unittest-data/testcases_common.dtsi    | 19 ++++
 .../of/unittest-data/tests-interrupts.dtsi    | 11 +--
 scripts/Makefile.lib                          | 40 ++++++--
 9 files changed, 218 insertions(+), 112 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

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
base-commit: a38fd8748464831584a19438cbb3082b5a2dab15
Does not apply to 5.12-rc2
I was based right over the 5.12-rc2 tag.

I don't know why I failed. Given your report, I went back to v5.12-rc2 and the patch series applies fine.

Either way, my tags are ok.

Quoted 7 lines

Quoted 4 lines
because of a dependency on a patch to scripts/Makefile.lib. That patch has been merged by Linus somewhere between -rc2 and -rc3.
git log --oneline v5.12-rc2..origin/master -- scripts/Makefile.lib

Quoted 7 lines

gives no results to me.
Quoted 2 lines
I had a working version
between -rc2 and -rc3 at commit e6f197677b2e
I have tried both Linus' tree and linux-next, and I don't see this commit.

Sorry about that, the commit id I gave was after applying the patch series. The commit id I should have reported is 144c79ef3353.

-Frank

Quoted 17 lines

Quoted 12 lines
that does have
the required patch, so that is the version I used to test this series.
There is still confusion caused by the contortions that unittest goes through to mis-use base DTBs vs overlay DTBs, so _after_ this series is merged by Rob, I will poke around and see if I can change unittest so that it does not look like it is mis-using DTBs and overlay DTBs.
Quoted 2 lines
Reviewed-by Frank Rowand <[email protected]>
Tested-by Frank Rowand <[email protected]>
Thanks.

Viresh Kumar

author

2021-03-12 04:33 UTC #34 lore raw

In reply to Viresh Kumar

Quoted 10 lines · Viresh Kumar

Hi,
This patchset adds a generic rule for applying overlays using fdtoverlay tool and then updates unittests to get built statically using the same.
V10->V11:
- Update patch 4/5 to fix checkpatch warning on spaces and tabs.
- Added Acked-by from Masahiro for patch 2/5.

Quoted 8 lines

Rob Herring (1):
kbuild: Add generic rule to apply fdtoverlay
Viresh Kumar (4):
kbuild: Simplify builds with CONFIG_OF_ALL_DTBS
kbuild: Allow .dtso format for overlay source files
of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
of: unittest: Statically apply overlays using fdtoverlay

Rob, can you please apply patch 1-2,4-5 from this series ? I will send a new version of the patch 3/5 (related to .dtso) separately.

Signature · 1 line

viresh

Rob Herring

maintainer Applied

2021-03-15 17:14 UTC #35 lore raw

In reply to Viresh Kumar

Quoted 61 lines · Viresh Kumar

Hi,
This patchset adds a generic rule for applying overlays using fdtoverlay tool and then updates unittests to get built statically using the same.
V10->V11:
- Update patch 4/5 to fix checkpatch warning on spaces and tabs.
- Added Acked-by from Masahiro for patch 2/5.
V9->V10:
- Add a new patch to allow .dtso files.
- Update 2/5 to be more efficient and also generate symbols for base files automatically.
- No need to add lines like DTC_FLAGS_foo_base += -@ in patch 5/5.
- Add Ack by Masahiro for 1/5.
V8->V9:
- Added some comment in patch 3/4 based on Frank's suggestions.
V7->V8:
- Patch 1 is new.
- Platforms need to use dtb-y += foo.dtb instead of overlay-y += foo.dtb.
- Use multi_depend instead of .SECONDEXPANSION.
- Use dtb-y for unittest instead of overlay-y.
- Rename the commented dtb filess in unittest Makefile as .dtbo.
- Improved Makefile code (I am learning a lot every day :)
V6->V7:
- Dropped the first 4 patches, already merged.
- Patch 1/3 is new, suggested by Rob and slightly modified by me.
- Adapt Patch 3/3 to the new rule and name the overlay dtbs as .dtbo.
--
Viresh
Rob Herring (1):
kbuild: Add generic rule to apply fdtoverlay
Viresh Kumar (4):
kbuild: Simplify builds with CONFIG_OF_ALL_DTBS
kbuild: Allow .dtso format for overlay source files
of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
of: unittest: Statically apply overlays using fdtoverlay
 drivers/of/unittest-data/Makefile             | 48 ++++++++++
 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        | 23 ++---
 .../of/unittest-data/testcases_common.dtsi    | 19 ++++
 .../of/unittest-data/tests-interrupts.dtsi    | 11 +--
 scripts/Makefile.lib                          | 40 ++++++--
 9 files changed, 218 insertions(+), 112 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

Quoted 6 lines

base-commit: a38fd8748464831584a19438cbb3082b5a2dab15 --
2.25.0.rc1.19.g042ed3e048af

Applied patches 1,2,4,5, thanks!