[PATCH V7 0/3] dt: Build unittests statically with fdtoverlay

[PATCH V7 0/3] dt: Build unittests statically with fdtoverlay

[Viresh Kumar]

Hi,

The first four patches from the previous patchset are already picked by
Rob for 5.12. This patchset contains the other two that update unittests
and an additional patch suggested by Rob.

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 (2):
  of: unittest: Create overlay_common.dtsi and testcases_common.dtsi
  of: unittest: Statically apply overlays using fdtoverlay

 drivers/of/unittest-data/Makefile             | 50 ++++++++++
 drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
 drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
 drivers/of/unittest-data/static_base_1.dts    |  4 +
 drivers/of/unittest-data/static_base_2.dts    |  4 +
 drivers/of/unittest-data/testcases.dts        | 18 ++--
 .../of/unittest-data/testcases_common.dtsi    | 19 ++++
 .../of/unittest-data/tests-interrupts.dtsi    |  7 --
 scripts/Makefile.lib                          | 39 +++++---
 9 files changed, 203 insertions(+), 119 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

-- 
2.25.0.rc1.19.g042ed3e048af

[PATCH V7 1/3] kbuild: Add generic rule to apply fdtoverlay

[Viresh Kumar]

From: Rob Herring <robh@kernel.org>

Add a generic rule to apply fdtoverlay in Makefile.lib, so every
platform doesn't need to carry the complex rule.

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

 DTC_FLAGS_foo_base += -@
 foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
 overlay-y := foo.dtb

Rearrange Makefile.lib to keep DT specific stuff together.

The files from overlay-y (i.e. files generated by fdtoverlay) aren't
added to dtb-y here, as dtb-y is later used to generate .dt.yaml files
and the files in overlay-y don't have a corresponding dts file and make
dtbs_check fails for them.

Signed-off-by: Rob Herring <robh@kernel.org>
[ Viresh: Add commit log and replace dtb-y with overlay-y, handle
          CONFIG_OF_ALL_DTBS case, rearrange Makefile, don't add
	  overlay-y to dtb-y to skip dtbs_check for them. ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 scripts/Makefile.lib | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index b00855b247e0..a6e79e3be527 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -66,23 +66,16 @@ multi-used   := $(multi-used-y) $(multi-used-m)
 real-obj-y := $(foreach m, $(obj-y), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)),$(m)))
 real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)),$(m)))
 
-always-y += $(always-m)
-
-# hostprogs-always-y += foo
-# ... is a shorthand for
-# hostprogs += foo
-# always-y  += foo
-hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
-always-y += $(hostprogs-always-y) $(hostprogs-always-m)
-
-# userprogs-always-y is likewise.
-userprogs += $(userprogs-always-y) $(userprogs-always-m)
-always-y += $(userprogs-always-y) $(userprogs-always-m)
+# Add base dtb and overlay dtbo
+dtb-y += $(foreach m,$(overlay-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
+dtb-$(CONFIG_OF_ALL_DTBS) += $(foreach m,$(overlay-), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
 
 # DTB
 # If CONFIG_OF_ALL_DTBS is enabled, all DT blobs are built
 extra-y				+= $(dtb-y)
+extra-y				+= $(overlay-y)
 extra-$(CONFIG_OF_ALL_DTBS)	+= $(dtb-)
+extra-$(CONFIG_OF_ALL_DTBS)	+= $(overlay-)
 
 ifneq ($(CHECK_DTBS),)
 extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
@@ -91,6 +84,19 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
 extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-))
 endif
 
+always-y += $(always-m)
+
+# hostprogs-always-y += foo
+# ... is a shorthand for
+# hostprogs += foo
+# always-y  += foo
+hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
+always-y += $(hostprogs-always-y) $(hostprogs-always-m)
+
+# userprogs-always-y is likewise.
+userprogs += $(userprogs-always-y) $(userprogs-always-m)
+always-y += $(userprogs-always-y) $(userprogs-always-m)
+
 # Add subdir path
 
 extra-y		:= $(addprefix $(obj)/,$(extra-y))
@@ -332,6 +338,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
 $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
 	$(call if_changed_dep,dtc)
 
+
+quiet_cmd_fdtoverlay = DTOVL   $@
+      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
+
+.SECONDEXPANSION:
+
+$(obj)/%.dtb: $$(addprefix $$(obj)/,$$(%-dtbs)) FORCE
+	$(call if_changed,fdtoverlay)
+
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
 # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
-- 
2.25.0.rc1.19.g042ed3e048af

[PATCH V7 2/3] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi

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

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

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

[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 <viresh.kumar@linaro.org>
---
 drivers/of/unittest-data/Makefile          | 50 ++++++++++++++++++++++
 drivers/of/unittest-data/static_base_1.dts |  4 ++
 drivers/of/unittest-data/static_base_2.dts |  4 ++
 3 files changed, 58 insertions(+)
 create mode 100644 drivers/of/unittest-data/static_base_1.dts
 create mode 100644 drivers/of/unittest-data/static_base_2.dts

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 009f4045c8e4..1d6029e722c0 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -34,7 +34,57 @@ DTC_FLAGS_overlay += -@
 DTC_FLAGS_overlay_bad_phandle += -@
 DTC_FLAGS_overlay_bad_symbol += -@
 DTC_FLAGS_overlay_base += -@
+DTC_FLAGS_static_base_1 += -@
+DTC_FLAGS_static_base_2 += -@
 DTC_FLAGS_testcases += -@
 
 # suppress warnings about intentional errors
 DTC_FLAGS_testcases += -Wno-interrupts_property
+
+# Apply overlays statically with fdtoverlay.  This is a build time test that
+# the overlays can be applied successfully by fdtoverlay.  This does not
+# guarantee that the overlays can be applied successfully at run time by
+# unittest, but it provides a bit of build time test coverage for those
+# who do not execute unittest.
+#
+# The overlays are applied on top of static_base_1.dtb and static_base_2.dtb to
+# create static_test_1.dtb and static_test_2.dtb.  If fdtoverlay detects an
+# error than the kernel build will fail.  static_test_1.dtb and
+# static_test_2.dtb are not consumed by unittest.
+#
+# Some unittest overlays deliberately contain errors that unittest checks for.
+# These overlays will cause fdtoverlay to fail, and are thus not included
+# in the static test:
+#			  overlay_bad_add_dup_node.dtb \
+#			  overlay_bad_add_dup_prop.dtb \
+#			  overlay_bad_phandle.dtb \
+#			  overlay_bad_symbol.dtb \
+
+apply_static_overlay_1 := overlay_0.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)
+
+overlay-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb
diff --git a/drivers/of/unittest-data/static_base_1.dts b/drivers/of/unittest-data/static_base_1.dts
new file mode 100644
index 000000000000..10556cb3f01f
--- /dev/null
+++ b/drivers/of/unittest-data/static_base_1.dts
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "testcases_common.dtsi"
diff --git a/drivers/of/unittest-data/static_base_2.dts b/drivers/of/unittest-data/static_base_2.dts
new file mode 100644
index 000000000000..b0ea9504d6f3
--- /dev/null
+++ b/drivers/of/unittest-data/static_base_2.dts
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "overlay_common.dtsi"
-- 
2.25.0.rc1.19.g042ed3e048af

Re: [PATCH V7 1/3] kbuild: Add generic rule to apply fdtoverlay

[Masahiro Yamada]

On Wed, Feb 10, 2021 at 8:13 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> From: Rob Herring <robh@kernel.org>
>
> Add a generic rule to apply fdtoverlay in Makefile.lib, so every
> platform doesn't need to carry the complex rule.
>
> The platform's Makefile only needs to have this now:
>
>  DTC_FLAGS_foo_base += -@
>  foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
>  overlay-y := foo.dtb


Please reuse dtb-y instead of introducing the new
overlay-y syntax, that is,

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



This resembles to composite modules.

foo-objs := foo1.o foo2.o foo3.o
obj-m := foo.o





> Rearrange Makefile.lib to keep DT specific stuff together.
>
> The files from overlay-y (i.e. files generated by fdtoverlay) aren't
> added to dtb-y here, as dtb-y is later used to generate .dt.yaml files
> and the files in overlay-y don't have a corresponding dts file and make
> dtbs_check fails for them.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> [ Viresh: Add commit log and replace dtb-y with overlay-y, handle
>           CONFIG_OF_ALL_DTBS case, rearrange Makefile, don't add
>           overlay-y to dtb-y to skip dtbs_check for them. ]
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  scripts/Makefile.lib | 39 +++++++++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index b00855b247e0..a6e79e3be527 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -66,23 +66,16 @@ multi-used   := $(multi-used-y) $(multi-used-m)
>  real-obj-y := $(foreach m, $(obj-y), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)),$(m)))
>  real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)),$(m)))
>
> -always-y += $(always-m)
> -
> -# hostprogs-always-y += foo
> -# ... is a shorthand for
> -# hostprogs += foo
> -# always-y  += foo
> -hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
> -always-y += $(hostprogs-always-y) $(hostprogs-always-m)
> -
> -# userprogs-always-y is likewise.
> -userprogs += $(userprogs-always-y) $(userprogs-always-m)
> -always-y += $(userprogs-always-y) $(userprogs-always-m)
> +# Add base dtb and overlay dtbo
> +dtb-y += $(foreach m,$(overlay-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
> +dtb-$(CONFIG_OF_ALL_DTBS) += $(foreach m,$(overlay-), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
>
>  # DTB
>  # If CONFIG_OF_ALL_DTBS is enabled, all DT blobs are built
>  extra-y                                += $(dtb-y)
> +extra-y                                += $(overlay-y)
>  extra-$(CONFIG_OF_ALL_DTBS)    += $(dtb-)
> +extra-$(CONFIG_OF_ALL_DTBS)    += $(overlay-)
>
>  ifneq ($(CHECK_DTBS),)
>  extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
> @@ -91,6 +84,19 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
>  extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-))
>  endif
>
> +always-y += $(always-m)
> +
> +# hostprogs-always-y += foo
> +# ... is a shorthand for
> +# hostprogs += foo
> +# always-y  += foo
> +hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
> +always-y += $(hostprogs-always-y) $(hostprogs-always-m)
> +
> +# userprogs-always-y is likewise.
> +userprogs += $(userprogs-always-y) $(userprogs-always-m)
> +always-y += $(userprogs-always-y) $(userprogs-always-m)
> +
>  # Add subdir path
>
>  extra-y                := $(addprefix $(obj)/,$(extra-y))
> @@ -332,6 +338,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
>  $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
>         $(call if_changed_dep,dtc)
>
> +
> +quiet_cmd_fdtoverlay = DTOVL   $@
> +      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
> +
> +.SECONDEXPANSION:
> +
> +$(obj)/%.dtb: $$(addprefix $$(obj)/,$$(%-dtbs)) FORCE
> +       $(call if_changed,fdtoverlay)
> +



Please do not use .SECONDEXPANSION.

This will parse the Makefile twice
in _all_ directories, while only a few
directories use the overlay-y syntax.


Use the multi_depend macro.






>  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

Re: [PATCH V7 1/3] kbuild: Add generic rule to apply fdtoverlay

[Rob Herring]

On Thu, Feb 11, 2021 at 12:28 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Feb 10, 2021 at 8:13 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > From: Rob Herring <robh@kernel.org>
> >
> > Add a generic rule to apply fdtoverlay in Makefile.lib, so every
> > platform doesn't need to carry the complex rule.
> >
> > The platform's Makefile only needs to have this now:
> >
> >  DTC_FLAGS_foo_base += -@
> >  foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
> >  overlay-y := foo.dtb
>
>
> Please reuse dtb-y instead of introducing the new
> overlay-y syntax, that is,
>
> foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
> dtb-y := foo.dtb

That's what I had, but I believe Viresh changed this because we don't
want to run schema checks on foo.dtb (as foo.dts doesn't exist).
However, we should be able to filter those out using something similar
to technique used for multi-used-y and real-obj-y. We just need to
drop composite entries when creating the .dt.yaml list.

> This resembles to composite modules.
>
> foo-objs := foo1.o foo2.o foo3.o
> obj-m := foo.o
>
>
>
>
>
> > Rearrange Makefile.lib to keep DT specific stuff together.

Would be better if you do this in a separate patch or not at all. It
was sorted more by the type of operation, than the target. Also, we
also already have one conflict in linux-next with changing extra- to
always-, let's not make it worse or this will be 5.13 for sure.

> > The files from overlay-y (i.e. files generated by fdtoverlay) aren't
> > added to dtb-y here, as dtb-y is later used to generate .dt.yaml files
> > and the files in overlay-y don't have a corresponding dts file and make
> > dtbs_check fails for them.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > [ Viresh: Add commit log and replace dtb-y with overlay-y, handle
> >           CONFIG_OF_ALL_DTBS case, rearrange Makefile, don't add
> >           overlay-y to dtb-y to skip dtbs_check for them. ]
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  scripts/Makefile.lib | 39 +++++++++++++++++++++++++++------------
> >  1 file changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index b00855b247e0..a6e79e3be527 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -66,23 +66,16 @@ multi-used   := $(multi-used-y) $(multi-used-m)
> >  real-obj-y := $(foreach m, $(obj-y), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)),$(m)))
> >  real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)),$(m)))
> >
> > -always-y += $(always-m)
> > -
> > -# hostprogs-always-y += foo
> > -# ... is a shorthand for
> > -# hostprogs += foo
> > -# always-y  += foo
> > -hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
> > -always-y += $(hostprogs-always-y) $(hostprogs-always-m)
> > -
> > -# userprogs-always-y is likewise.
> > -userprogs += $(userprogs-always-y) $(userprogs-always-m)
> > -always-y += $(userprogs-always-y) $(userprogs-always-m)
> > +# Add base dtb and overlay dtbo
> > +dtb-y += $(foreach m,$(overlay-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
> > +dtb-$(CONFIG_OF_ALL_DTBS) += $(foreach m,$(overlay-), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),))
> >
> >  # DTB
> >  # If CONFIG_OF_ALL_DTBS is enabled, all DT blobs are built
> >  extra-y                                += $(dtb-y)
> > +extra-y                                += $(overlay-y)
> >  extra-$(CONFIG_OF_ALL_DTBS)    += $(dtb-)
> > +extra-$(CONFIG_OF_ALL_DTBS)    += $(overlay-)
> >
> >  ifneq ($(CHECK_DTBS),)
> >  extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
> > @@ -91,6 +84,19 @@ extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
> >  extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-))
> >  endif
> >
> > +always-y += $(always-m)
> > +
> > +# hostprogs-always-y += foo
> > +# ... is a shorthand for
> > +# hostprogs += foo
> > +# always-y  += foo
> > +hostprogs += $(hostprogs-always-y) $(hostprogs-always-m)
> > +always-y += $(hostprogs-always-y) $(hostprogs-always-m)
> > +
> > +# userprogs-always-y is likewise.
> > +userprogs += $(userprogs-always-y) $(userprogs-always-m)
> > +always-y += $(userprogs-always-y) $(userprogs-always-m)
> > +
> >  # Add subdir path
> >
> >  extra-y                := $(addprefix $(obj)/,$(extra-y))
> > @@ -332,6 +338,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> >  $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
> >         $(call if_changed_dep,dtc)
> >
> > +
> > +quiet_cmd_fdtoverlay = DTOVL   $@
> > +      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs)
> > +
> > +.SECONDEXPANSION:
> > +
> > +$(obj)/%.dtb: $$(addprefix $$(obj)/,$$(%-dtbs)) FORCE
> > +       $(call if_changed,fdtoverlay)
> > +
>
>
>
> Please do not use .SECONDEXPANSION.
>
> This will parse the Makefile twice
> in _all_ directories, while only a few
> directories use the overlay-y syntax.
>
>
> Use the multi_depend macro.

I knew you'd have a better way.

Rob

Re: [PATCH V7 1/3] kbuild: Add generic rule to apply fdtoverlay

[Masahiro Yamada]

On Fri, Feb 12, 2021 at 5:05 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Feb 11, 2021 at 12:28 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Feb 10, 2021 at 8:13 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > From: Rob Herring <robh@kernel.org>
> > >
> > > Add a generic rule to apply fdtoverlay in Makefile.lib, so every
> > > platform doesn't need to carry the complex rule.
> > >
> > > The platform's Makefile only needs to have this now:
> > >
> > >  DTC_FLAGS_foo_base += -@
> > >  foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
> > >  overlay-y := foo.dtb
> >
> >
> > Please reuse dtb-y instead of introducing the new
> > overlay-y syntax, that is,
> >
> > foo-dtbs := foo_base.dtb foo_overlay1.dtbo foo_overlay2.dtbo
> > dtb-y := foo.dtb
>
> That's what I had, but I believe Viresh changed this because we don't
> want to run schema checks on foo.dtb (as foo.dts doesn't exist).
> However, we should be able to filter those out using something similar
> to technique used for multi-used-y and real-obj-y. We just need to
> drop composite entries when creating the .dt.yaml list.



Yes, I think this will work.



BTW, I do not know how to use overlay.
Do we apply overlay in the build time?
If so, I do not know what the benefit of overlay is.
Or is this just for build testing?


I just thought this was done in the boot time,
for example, in U-Boot or something.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH V7 1/3] kbuild: Add generic rule to apply fdtoverlay

[Viresh Kumar]

On 12-02-21, 12:07, Masahiro Yamada wrote:
> BTW, I do not know how to use overlay.
> Do we apply overlay in the build time?

Ideally it can be applied at both build time and runtime, but we
haven't allowed the runtime way until now in kernel. This patchset is
all about applying it at build time.

> If so, I do not know what the benefit of overlay is.
> Or is this just for build testing?

For now the main benefit of using them is that we can keep stuff in
separate files without including each other. For example a primary
board may or may not have an extension board connected to it.

Without overlays we will have this many dtbs for this simple case:
1. primary.dtb
2. extension.dtb
3. primary-includes-extension.dtb

With overlays we will have the first two. Now the same extension can
be applied to lots of boards and multiple extensions can be applied to
the same primary board. This just complicates the process of managing
dtbs.

> I just thought this was done in the boot time,
> for example, in U-Boot or something.

Yes, bootloader can do it as well.

-- 
viresh