[PATCH V11 0/5] dt: Add fdtoverlay rule and statically build unittest

[PATCH V11 0/5] dt: Add fdtoverlay rule and statically build unittest

[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


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

[PATCH V11 1/5] kbuild: Simplify builds with CONFIG_OF_ALL_DTBS

[Viresh Kumar]

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 <masahiroy@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 scripts/Makefile.lib | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index eee59184de64..a2658242d956 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -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
-- 
2.25.0.rc1.19.g042ed3e048af

[PATCH V11 2/5] 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. 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 <masahiroy@kernel.org>
Signed-off-by: Rob Herring <robh@kernel.org>
Co-developed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 scripts/Makefile.lib | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index a2658242d956..bc045a54a34e 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -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
-- 
2.25.0.rc1.19.g042ed3e048af

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

[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 <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 scripts/Makefile.lib | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index bc045a54a34e..59e86f67f9e0 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -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
-- 
2.25.0.rc1.19.g042ed3e048af

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

[Masahiro Yamada]

On Wed, Mar 10, 2021 at 2:35 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> 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 <geert+renesas@glider.be>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  scripts/Makefile.lib | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index bc045a54a34e..59e86f67f9e0 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -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/20210310110824.782209-1-masahiroy@kernel.org/

I will queue it to linux-kbuild/fixes.



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






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


-- 
Best Regards
Masahiro Yamada

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

[Masahiro Yamada]

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

On Wed, Mar 10, 2021 at 8:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Mar 10, 2021 at 2:35 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > 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 <geert+renesas@glider.be>
> > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  scripts/Makefile.lib | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index bc045a54a34e..59e86f67f9e0 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -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/20210310110824.782209-1-masahiroy@kernel.org/
>
> 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.


-- 
Best Regards
Masahiro Yamada

[-- Attachment #2: 0001-dtc-Remove-O-dtbo-support.patch --]
[-- Type: text/x-patch, Size: 927 bytes --]

From eeac710289a22397c6ae4e90a9334c4f7e178688 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <masahiroy@kernel.org>
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 <masahiroy@kernel.org>
---
 dtc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/dtc.c b/dtc.c
index 838c5df..3962d3f 100644
--- a/dtc.c
+++ b/dtc.c
@@ -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")) {
-- 
2.27.0

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

[Viresh Kumar]

On 10-03-21, 20:29, Masahiro Yamada wrote:
> 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.

-- 
viresh

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

[Masahiro Yamada]

On Wed, Mar 10, 2021 at 11:48 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 10-03-21, 20:29, Masahiro Yamada wrote:
> > 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.



-- 
Best Regards
Masahiro Yamada

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

[Viresh Kumar]

On 11-03-21, 00:18, Masahiro Yamada wrote:
> On Wed, Mar 10, 2021 at 11:48 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 10-03-21, 20:29, Masahiro Yamada wrote:
> > > 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.

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.

-- 
viresh

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

[Viresh Kumar]

On 10-03-21, 20:24, Masahiro Yamada wrote:
> On Wed, Mar 10, 2021 at 2:35 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index bc045a54a34e..59e86f67f9e0 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -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,

I missed this TBH.

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

That can be a reason now :)

> I will drop the ugly -O.
> https://patchwork.kernel.org/project/linux-kbuild/patch/20210310110824.782209-1-masahiroy@kernel.org/

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

> I will queue it to linux-kbuild/fixes.
> 
> 
> 
> >                 $(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?

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

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

[Masahiro Yamada]

On Wed, Mar 10, 2021 at 11:47 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 10-03-21, 20:24, Masahiro Yamada wrote:
> > On Wed, Mar 10, 2021 at 2:35 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > > index bc045a54a34e..59e86f67f9e0 100644
> > > --- a/scripts/Makefile.lib
> > > +++ b/scripts/Makefile.lib
> > > @@ -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,
>
> I missed this TBH.
>
> > but I guess you wanted to make this explicit, correct?
>
> That can be a reason now :)
>
> > I will drop the ugly -O.
> > https://patchwork.kernel.org/project/linux-kbuild/patch/20210310110824.782209-1-masahiroy@kernel.org/
>
> But if we are going to depend on DTC to guess it right, then we
> shouldn't add -I at all..
>
> > I will queue it to linux-kbuild/fixes.
> >
> >
> >
> > >                 $(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?
>
> 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" ?




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




-- 
Best Regards
Masahiro Yamada

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

[Frank Rowand]

On 3/10/21 9:15 AM, Masahiro Yamada wrote:
> On Wed, Mar 10, 2021 at 11:47 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>
>> On 10-03-21, 20:24, Masahiro Yamada wrote:
>>> On Wed, Mar 10, 2021 at 2:35 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>>> index bc045a54a34e..59e86f67f9e0 100644
>>>> --- a/scripts/Makefile.lib
>>>> +++ b/scripts/Makefile.lib
>>>> @@ -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,
>>
>> I missed this TBH.
>>
>>> but I guess you wanted to make this explicit, correct?
>>
>> That can be a reason now :)
>>
>>> I will drop the ugly -O.
>>> https://patchwork.kernel.org/project/linux-kbuild/patch/20210310110824.782209-1-masahiroy@kernel.org/
>>
>> But if we are going to depend on DTC to guess it right, then we
>> shouldn't add -I at all..
>>
>>> I will queue it to linux-kbuild/fixes.
>>>
>>>
>>>
>>>>                 $(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?
>>
>> 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" ?
> 
> 
> 
> 
>> 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.

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

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

[Viresh Kumar]

On 10-03-21, 20:24, Masahiro Yamada wrote:
> 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?


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

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index a5d2d9254b2c..9f3426ec3fab 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -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)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index bc045a54a34e..77a9be055e51 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -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

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

[Frank Rowand]

Hi Viresh,

On 3/11/21 10:47 PM, Viresh Kumar wrote:
> On 10-03-21, 20:24, Masahiro Yamada wrote:
>> 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?
> 
> 
>>> +# 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.

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

> 
> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
> index a5d2d9254b2c..9f3426ec3fab 100644
> --- a/drivers/of/unittest-data/Makefile
> +++ b/drivers/of/unittest-data/Makefile
> @@ -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)
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index bc045a54a34e..77a9be055e51 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -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
> 

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

[Frank Rowand]

On 3/12/21 1:03 AM, Frank Rowand wrote:
> Hi Viresh,
> 
> On 3/11/21 10:47 PM, Viresh Kumar wrote:
>> On 10-03-21, 20:24, Masahiro Yamada wrote:
>>> 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?
>>
>>
>>>> +# 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.
> 
> 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

> 
> 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
> 
>>
>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
>> index a5d2d9254b2c..9f3426ec3fab 100644
>> --- a/drivers/of/unittest-data/Makefile
>> +++ b/drivers/of/unittest-data/Makefile
>> @@ -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)
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index bc045a54a34e..77a9be055e51 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -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
>>
> 

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

[Viresh Kumar]

On 12-03-21, 01:09, Frank Rowand wrote:
> 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 ?

-- 
viresh

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

[Frank Rowand]

On 3/12/21 1:13 AM, Viresh Kumar wrote:
> On 12-03-21, 01:09, Frank Rowand wrote:
>> 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

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

[Frank Rowand]

Hi VIresh,

On 3/12/21 11:11 PM, Frank Rowand wrote:
> On 3/12/21 1:13 AM, Viresh Kumar wrote:
>> On 12-03-21, 01:09, Frank Rowand wrote:
>>> 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).

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

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

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

[Viresh Kumar]

On 14-03-21, 20:16, Frank Rowand wrote:
> On 3/12/21 11:11 PM, Frank Rowand wrote:
> > On 3/12/21 1:13 AM, Viresh Kumar wrote:
> >> On 12-03-21, 01:09, Frank Rowand wrote:
> >>> 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).
> 
> 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)

-- 
viresh

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

[Masahiro Yamada]

On Mon, Mar 15, 2021 at 3:40 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 14-03-21, 20:16, Frank Rowand wrote:
> > On 3/12/21 11:11 PM, Frank Rowand wrote:
> > > On 3/12/21 1:13 AM, Viresh Kumar wrote:
> > >> On 12-03-21, 01:09, Frank Rowand wrote:
> > >>> 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).
> >
> > 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)
>
> --
> 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





-- 
Best Regards
Masahiro Yamada

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

[Laurent Pinchart]

Hi Yamada-san,

On Tue, Mar 16, 2021 at 02:43:45AM +0900, Masahiro Yamada wrote:
> On Mon, Mar 15, 2021 at 3:40 PM Viresh Kumar wrote:
> > On 14-03-21, 20:16, Frank Rowand wrote:
> > > On 3/12/21 11:11 PM, Frank Rowand wrote:
> > > > On 3/12/21 1:13 AM, Viresh Kumar wrote:
> > > >> On 12-03-21, 01:09, Frank Rowand wrote:
> > > >>> 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).
> > >
> > > 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)
> >
> 
> 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 :-)

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

-- 
Regards,

Laurent Pinchart

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

[Frank Rowand]

On 3/15/21 5:12 PM, Laurent Pinchart wrote:
> Hi Yamada-san,
> 
> On Tue, Mar 16, 2021 at 02:43:45AM +0900, Masahiro Yamada wrote:
>> On Mon, Mar 15, 2021 at 3:40 PM Viresh Kumar wrote:
>>> On 14-03-21, 20:16, Frank Rowand wrote:
>>>> On 3/12/21 11:11 PM, Frank Rowand wrote:
>>>>> On 3/12/21 1:13 AM, Viresh Kumar wrote:
>>>>>> On 12-03-21, 01:09, Frank Rowand wrote:
>>>>>>> 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).
>>>>
>>>> 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)
>>>
>>
>> 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).

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

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

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

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

[Geert Uytterhoeven]

Hi Frank,

On Tue, Mar 16, 2021 at 6:39 AM Frank Rowand <frowand.list@gmail.com> wrote:
> On 3/15/21 5:12 PM, Laurent Pinchart wrote:
> > On Tue, Mar 16, 2021 at 02:43:45AM +0900, Masahiro Yamada wrote:
> >> 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.
>
> > 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.

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?

> >> 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 -- geert@linux-m68k.org

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

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

[Masahiro Yamada]

On Tue, Mar 16, 2021 at 5:01 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Frank,
>
> On Tue, Mar 16, 2021 at 6:39 AM Frank Rowand <frowand.list@gmail.com> wrote:
> > On 3/15/21 5:12 PM, Laurent Pinchart wrote:
> > > On Tue, Mar 16, 2021 at 02:43:45AM +0900, Masahiro Yamada wrote:
> > >> 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.
> >
> > > 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.
>
> 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?
>
> > >> 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")


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

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

[Viresh Kumar]

On 16-03-21, 02:43, Masahiro Yamada wrote:
> On Mon, Mar 15, 2021 at 3:40 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > 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 ?
> >
> > +# 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.

> 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

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.

-- 
viresh

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

[Frank Rowand]

On 3/15/21 1:40 AM, Viresh Kumar wrote:
> On 14-03-21, 20:16, Frank Rowand wrote:
>> On 3/12/21 11:11 PM, Frank Rowand wrote:
>>> On 3/12/21 1:13 AM, Viresh Kumar wrote:
>>>> On 12-03-21, 01:09, Frank Rowand wrote:
>>>>> 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).
>>
>> 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)
> 

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

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

[Viresh Kumar]

On 16-03-21, 00:36, Frank Rowand wrote:
> 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.

-- 
viresh

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

[Frank Rowand]

On 3/16/21 12:42 AM, Viresh Kumar wrote:
> On 16-03-21, 00:36, Frank Rowand wrote:
>> 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.
> 

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

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

[Frank Rowand]

Hi Viresh,

On 3/24/21 2:34 AM, Frank Rowand wrote:
> On 3/16/21 12:42 AM, Viresh Kumar wrote:
>> On 16-03-21, 00:36, Frank Rowand wrote:
>>> 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.
>>
> 
> 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.

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/20210324223713.1334666-1-frowand.list@gmail.com

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

-Frank

[PATCH V11 4/5] 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        | 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

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..61cdd3d5fccb 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -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"
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..9b60a549f502 100644
--- a/drivers/of/unittest-data/tests-interrupts.dtsi
+++ b/drivers/of/unittest-data/tests-interrupts.dtsi
@@ -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.
+		 */
 	};
-
 };
-- 
2.25.0.rc1.19.g042ed3e048af

[PATCH V11 5/5] 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          | 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

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 009f4045c8e4..a5d2d9254b2c 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -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
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 V11 0/5] dt: Add fdtoverlay rule and statically build unittest

[Frank Rowand]

On 3/9/21 11:35 PM, Viresh Kumar wrote:
> 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
> 

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 <frank.rowand@sony.com>
Tested-by: Frank Rowand <frank.rowand@sony.com>

Re: [PATCH V11 0/5] dt: Add fdtoverlay rule and statically build unittest

[Viresh Kumar]

On 11-03-21, 17:27, Frank Rowand wrote:
> On 3/9/21 11:35 PM, Viresh Kumar wrote:
> > 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
> > 
> 
> Does not apply to 5.12-rc2

I was based right over the 5.12-rc2 tag.

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

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

> 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 <frank.rowand@sony.com>
> Tested-by: Frank Rowand <frank.rowand@sony.com>

Thanks.

-- 
viresh

Re: [PATCH V11 0/5] dt: Add fdtoverlay rule and statically build unittest

[Frank Rowand]

On 3/11/21 10:31 PM, Viresh Kumar wrote:
> On 11-03-21, 17:27, Frank Rowand wrote:
>> On 3/9/21 11:35 PM, Viresh Kumar wrote:
>>> 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
>>>
>>
>> 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.

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

> 
>> 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 <frank.rowand@sony.com>
>> Tested-by: Frank Rowand <frank.rowand@sony.com>
> 
> Thanks.
> 

Re: [PATCH V11 0/5] dt: Add fdtoverlay rule and statically build unittest

[Viresh Kumar]

On 10-03-21, 11:05, Viresh Kumar wrote:
> 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.
> 
> 
> 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.

-- 
viresh

Re: [PATCH V11 0/5] dt: Add fdtoverlay rule and statically build unittest

[Rob Herring]

On Wed, 10 Mar 2021 11:05:28 +0530, Viresh Kumar wrote:
> 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
> --
> 2.25.0.rc1.19.g042ed3e048af
> 
> 
> 

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