[PATCH V7 0/6] dt: build overlays

[PATCH V7 0/6] dt: build overlays

[Viresh Kumar]

Hi,

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

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

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

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

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

V5:

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

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

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

--
Viresh

[1] https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc
[2] https://lore.kernel.org/lkml/74f8aa8f-ffab-3b0f-186f-31fb7395ebbb@gmail.com/

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

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


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

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

[Viresh Kumar]

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

Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .gitignore               | 1 +
 Makefile                 | 5 ++++-
 scripts/Makefile.dtbinst | 3 +++
 scripts/Makefile.lib     | 5 +++++
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index d01cda8e1177..bb65fa253e58 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,6 +18,7 @@
 *.c.[012]*.*
 *.dt.yaml
 *.dtb
+*.dtbo
 *.dtb.S
 *.dwo
 *.elf
diff --git a/Makefile b/Makefile
index e0af7a4a5598..d5bc67e523be 100644
--- a/Makefile
+++ b/Makefile
@@ -1337,6 +1337,9 @@ ifneq ($(dtstree),)
 %.dtb: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
+%.dtbo: include/config/kernel.release scripts_dtc
+	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
+
 PHONY += dtbs dtbs_install dtbs_check
 dtbs: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree)
@@ -1816,7 +1819,7 @@ clean: $(clean-dirs)
 	@find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \
 		\( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \
 		-o -name '*.ko.*' \
-		-o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
+		-o -name '*.dtb' -o -name '*.dtbo' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
 		-o -name '*.dwo' -o -name '*.lst' \
 		-o -name '*.su' -o -name '*.mod' \
 		-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
diff --git a/scripts/Makefile.dtbinst b/scripts/Makefile.dtbinst
index 50d580d77ae9..ba01f5ba2517 100644
--- a/scripts/Makefile.dtbinst
+++ b/scripts/Makefile.dtbinst
@@ -29,6 +29,9 @@ quiet_cmd_dtb_install = INSTALL $@
 $(dst)/%.dtb: $(obj)/%.dtb
 	$(call cmd,dtb_install)
 
+$(dst)/%.dtbo: $(obj)/%.dtbo
+	$(call cmd,dtb_install)
+
 PHONY += $(subdirs)
 $(subdirs):
 	$(Q)$(MAKE) $(dtbinst)=$@ dst=$(patsubst $(obj)/%,$(dst)/%,$@)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 213677a5ed33..b00855b247e0 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -86,7 +86,9 @@ extra-$(CONFIG_OF_ALL_DTBS)	+= $(dtb-)
 
 ifneq ($(CHECK_DTBS),)
 extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
+extra-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y))
 extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
+extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-))
 endif
 
 # Add subdir path
@@ -327,6 +329,9 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
 $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
 	$(call if_changed_dep,dtc)
 
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+	$(call if_changed_dep,dtc)
+
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
 # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
-- 
2.25.0.rc1.19.g042ed3e048af

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

[Rob Herring]

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

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

Rob

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

[Geert Uytterhoeven]

Hi Viresh,

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

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

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

> .dtbo extension to distinguish it from normal blobs.
>
> Acked-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

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 V7 4/6] kbuild: Add support to build overlays (%.dtbo)

[Viresh Kumar]

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

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

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

-- 
viresh

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

[Geert Uytterhoeven]

Hi Viresh,

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

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 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 V7 4/6] kbuild: Add support to build overlays (%.dtbo)

[Viresh Kumar]

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

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

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

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

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

-- 
viresh

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

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

[Geert Uytterhoeven]

Hi Viresh,

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

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

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

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

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

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 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 V7 4/6] kbuild: Add support to build overlays (%.dtbo)

[Rob Herring]

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

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

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

Rob

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

[David Gibson]

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

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

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

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

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

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

[Geert Uytterhoeven]

Hi Rob,

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

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

Which is not needed with the kernel build rules.

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

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

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

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

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

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

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

    obj-$(CONFIG_FOO) += foo.dtbo

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 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 V7 4/6] kbuild: Add support to build overlays (%.dtbo)

[Frank Rowand]

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

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

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

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

-Frank

> 
> Rob
> 

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

[Viresh Kumar]

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

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

What about this ?

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

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

I had to keep the original line as is:

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

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

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

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

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

- Something else ?

-- 
viresh

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

[Geert Uytterhoeven]

Hi Viresh,

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

Thanks, looks good to me, and works for me, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

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

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

Rename unittest .dts files to .dtso where applicable?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 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 V7 4/6] kbuild: Add support to build overlays (%.dtbo)

[Viresh Kumar]

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

Thanks.

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

I will let Rob answer that :)

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

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

-- 
viresh