linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
@ 2021-01-05 11:24 Viresh Kumar
  2021-01-05 11:24 ` [RFC 1/2] " Viresh Kumar
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Viresh Kumar @ 2021-01-05 11:24 UTC (permalink / raw)
  To: Pantelis Antoniou, Frank Rowand, Rob Herring, Masahiro Yamada,
	Michal Marek
  Cc: Viresh Kumar, devicetree, linux-kernel, linux-kbuild,
	Vincent Guittot, Bill Mills, tero.kristo

Hello,

Here is an attempt to make some changes in the kernel to allow building
of device tree overlays.

While at it, I would also like to discuss about how we should mention
the base DT blobs in the Makefiles for the overlays, so they can be
build tested to make sure the overlays apply properly.

A simple way is to mention that with -base extension, like this:

$(overlay-file)-base := platform-base.dtb

Any other preference ?

Also fdtoverlay is an external entity right now, and is not part of the
kernel. Do we need to make it part of the kernel ? Or keep using the
external entity ?

Thanks.

--
Viresh

Viresh Kumar (2):
  kbuild: Add support to build overlays (%.dtbo)
  scripts: dtc: Handle outform dtbo

 Makefile             |  4 ++--
 scripts/Makefile.lib | 12 ++++++++++++
 scripts/dtc/dtc.c    |  2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)

-- 
2.25.0.rc1.19.g042ed3e048af


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

* [RFC 1/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-05 11:24 [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
@ 2021-01-05 11:24 ` Viresh Kumar
  2021-01-05 11:24 ` [RFC 2/2] scripts: dtc: Handle outform dtbo Viresh Kumar
  2021-01-05 15:21 ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Rob Herring
  2 siblings, 0 replies; 20+ messages in thread
From: Viresh Kumar @ 2021-01-05 11:24 UTC (permalink / raw)
  To: Pantelis Antoniou, Frank Rowand, Rob Herring, Masahiro Yamada,
	Michal Marek
  Cc: Viresh Kumar, devicetree, linux-kernel, linux-kbuild,
	Vincent Guittot, Bill Mills, tero.kristo

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

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Makefile             |  4 ++--
 scripts/Makefile.lib | 12 ++++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 3d328b7ab200..54cdcdfea67b 100644
--- a/Makefile
+++ b/Makefile
@@ -1334,7 +1334,7 @@ endif
 
 ifneq ($(dtstree),)
 
-%.dtb: include/config/kernel.release scripts_dtc
+%.dtb %.dtbo: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
 PHONY += dtbs dtbs_install dtbs_check
@@ -1816,7 +1816,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.lib b/scripts/Makefile.lib
index 213677a5ed33..f70d7bd3262a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -82,11 +82,15 @@ always-y += $(userprogs-always-y) $(userprogs-always-m)
 # DTB
 # If CONFIG_OF_ALL_DTBS is enabled, all DT blobs are built
 extra-y				+= $(dtb-y)
+extra-y				+= $(dtbo-y)
 extra-$(CONFIG_OF_ALL_DTBS)	+= $(dtb-)
+extra-$(CONFIG_OF_ALL_DTBS)	+= $(dtbo-)
 
 ifneq ($(CHECK_DTBS),)
 extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
+extra-y += $(patsubst %.dtbo,%.dt.yaml, $(dtbo-y))
 extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
+extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtbo-))
 endif
 
 # Add subdir path
@@ -299,6 +303,11 @@ endif
 
 DTC_FLAGS += $(DTC_FLAGS_$(basetarget))
 
+# enable creation of __symbols__ node
+ifneq ($(dtbo-y),)
+DTC_FLAGS += -@
+endif
+
 # Generate an assembly file to wrap the output of the device tree compiler
 quiet_cmd_dt_S_dtb= DTB     $@
 cmd_dt_S_dtb=						\
@@ -327,6 +336,9 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
 $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
 	$(call if_changed_dep,dtc)
 
+$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
+	$(call if_changed_dep,dtc)
+
 DT_CHECKER ?= dt-validate
 DT_BINDING_DIR := Documentation/devicetree/bindings
 # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
-- 
2.25.0.rc1.19.g042ed3e048af


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

* [RFC 2/2] scripts: dtc: Handle outform dtbo
  2021-01-05 11:24 [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
  2021-01-05 11:24 ` [RFC 1/2] " Viresh Kumar
@ 2021-01-05 11:24 ` Viresh Kumar
  2021-01-05 15:37   ` Rob Herring
  2021-01-05 15:21 ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Rob Herring
  2 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2021-01-05 11:24 UTC (permalink / raw)
  To: Pantelis Antoniou, Frank Rowand, Rob Herring
  Cc: Viresh Kumar, devicetree, linux-kernel, linux-kbuild,
	Vincent Guittot, Bill Mills, tero.kristo

Update dtc compiler to accept dtbo as an outform.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
I feel that this needs to go directly to
https://git.kernel.org/pub/scm/utils/dtc/dtc.git

Right ? I will send it separately if the idea is accepted here.
---
 scripts/dtc/dtc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
index bdb3f5945699..40fa7128b3d6 100644
--- a/scripts/dtc/dtc.c
+++ b/scripts/dtc/dtc.c
@@ -357,6 +357,8 @@ 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.25.0.rc1.19.g042ed3e048af


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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-05 11:24 [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
  2021-01-05 11:24 ` [RFC 1/2] " Viresh Kumar
  2021-01-05 11:24 ` [RFC 2/2] scripts: dtc: Handle outform dtbo Viresh Kumar
@ 2021-01-05 15:21 ` Rob Herring
  2021-01-06 10:09   ` [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump Viresh Kumar
                     ` (2 more replies)
  2 siblings, 3 replies; 20+ messages in thread
From: Rob Herring @ 2021-01-05 15:21 UTC (permalink / raw)
  To: Viresh Kumar, Arnd Bergmann, Olof Johansson
  Cc: Pantelis Antoniou, Frank Rowand, Masahiro Yamada, Michal Marek,
	devicetree, linux-kernel, Linux Kbuild mailing list,
	Vincent Guittot, Bill Mills, tero.kristo

On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Hello,
>
> Here is an attempt to make some changes in the kernel to allow building
> of device tree overlays.
>
> While at it, I would also like to discuss about how we should mention
> the base DT blobs in the Makefiles for the overlays, so they can be
> build tested to make sure the overlays apply properly.
>
> A simple way is to mention that with -base extension, like this:
>
> $(overlay-file)-base := platform-base.dtb
>
> Any other preference ?

I think we'll want something similar to how '-objs' works for modules:

foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo
foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo
foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo
dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb

(One difference here is we will want all the intermediate targets
unlike .o files.)

You wouldn't necessarily have all the above combinations, but you have
to allow for them. I'm not sure how we'd handle applying any common
overlays where the base and overlay are in different directories.

Another thing here is adding all the above is not really going to
scale on arm32 where we have a single dts directory. We need to move
things to per vendor/soc family directories. I have the script to do
this. We just need to agree on the vendor names and get Arnd/Olof to
run it. I also want that so we can enable schema checks by default
once a vendor is warning free (the whole tree is going to take
forever).

> Also fdtoverlay is an external entity right now, and is not part of the
> kernel. Do we need to make it part of the kernel ? Or keep using the
> external entity ?

Part of the kernel. We just need to add it to the dtc sync script and
makefile I think.

Rob

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

* Re: [RFC 2/2] scripts: dtc: Handle outform dtbo
  2021-01-05 11:24 ` [RFC 2/2] scripts: dtc: Handle outform dtbo Viresh Kumar
@ 2021-01-05 15:37   ` Rob Herring
  2021-01-12  0:18     ` Frank Rowand
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-01-05 15:37 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Pantelis Antoniou, Frank Rowand, devicetree, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	tero.kristo

On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Update dtc compiler to accept dtbo as an outform.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> ---
> I feel that this needs to go directly to
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git
>
> Right ? I will send it separately if the idea is accepted here.

Yes, needs to go to devicetree-compiler list. I think this came up
before and IIRC David wasn't completely in agreement. I looked briefly
and couldn't find the thread though...

We really don't need a different extension because we could just
examine the dtb to determine if it is an overlay or not. That's less
obvious. We could also add meta-data to overlays defining what base
they apply to. If we had that, a tool could just list all overlays
that should apply to a base and we could use that info for build time
applying overlays. Of course, that and a dtbo extension/format are not
mutually exclusive.

> ---
>  scripts/dtc/dtc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
> index bdb3f5945699..40fa7128b3d6 100644
> --- a/scripts/dtc/dtc.c
> +++ b/scripts/dtc/dtc.c
> @@ -357,6 +357,8 @@ 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")) {

You also need to extend guess_type_by_name().


Rob

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

* [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump
  2021-01-05 15:21 ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Rob Herring
@ 2021-01-06 10:09   ` Viresh Kumar
  2021-01-06 12:24     ` kernel test robot
  2021-01-06 15:52     ` Rob Herring
  2021-01-07  5:28   ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Masahiro Yamada
  2021-01-12  0:25   ` Frank Rowand
  2 siblings, 2 replies; 20+ messages in thread
From: Viresh Kumar @ 2021-01-06 10:09 UTC (permalink / raw)
  To: Pantelis Antoniou, Frank Rowand, Rob Herring
  Cc: Viresh Kumar, devicetree, linux-kernel, linux-kbuild,
	Vincent Guittot, Bill Mills, anmar.oueja

We will start building overlays for platforms soon in the kernel and
would need these tools going forward. Lets fetch and build these.

Note that a copy of fdtdump.c was already copied back in the year 2012,
but it was never updated or built for some reason.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 scripts/dtc/Makefile             | 8 +++++++-
 scripts/dtc/update-dtc-source.sh | 6 +++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 4852bf44e913..c607980a5c17 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -1,12 +1,18 @@
 # SPDX-License-Identifier: GPL-2.0
 # scripts/dtc makefile
 
-hostprogs-always-$(CONFIG_DTC)		+= dtc
+hostprogs-always-$(CONFIG_DTC)		+= dtc fdtdump fdtoverlay
 hostprogs-always-$(CHECK_DT_BINDING)	+= dtc
 
 dtc-objs	:= dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
 		   srcpos.o checks.o util.o
 dtc-objs	+= dtc-lexer.lex.o dtc-parser.tab.o
+fdtdump-objs	:= fdtdump.o util.o
+
+libfdt_dir	= libfdt
+libfdt-objs	:= fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o
+libfdt		= $(addprefix $(libfdt_dir)/,$(libfdt-objs))
+fdtoverlay-objs	:= $(libfdt) fdtoverlay.o util.o
 
 # Source files need to get at the userspace version of libfdt_env.h to compile
 HOST_EXTRACFLAGS += -I $(srctree)/$(src)/libfdt
diff --git a/scripts/dtc/update-dtc-source.sh b/scripts/dtc/update-dtc-source.sh
index bc704e2a6a4a..9bc4afb71415 100755
--- a/scripts/dtc/update-dtc-source.sh
+++ b/scripts/dtc/update-dtc-source.sh
@@ -31,9 +31,9 @@ set -ev
 DTC_UPSTREAM_PATH=`pwd`/../dtc
 DTC_LINUX_PATH=`pwd`/scripts/dtc
 
-DTC_SOURCE="checks.c data.c dtc.c dtc.h flattree.c fstree.c livetree.c srcpos.c \
-		srcpos.h treesource.c util.c util.h version_gen.h yamltree.c \
-		dtc-lexer.l dtc-parser.y"
+DTC_SOURCE="checks.c data.c dtc.c dtc.h fdtdump.c fdtoverlay.c flattree.c \
+		fstree.c livetree.c srcpos.c srcpos.h treesource.c util.c \
+		util.h version_gen.h yamltree.c dtc-lexer.l dtc-parser.y"
 LIBFDT_SOURCE="fdt.c fdt.h fdt_addresses.c fdt_empty_tree.c \
 		fdt_overlay.c fdt_ro.c fdt_rw.c fdt_strerror.c fdt_sw.c \
 		fdt_wip.c libfdt.h libfdt_env.h libfdt_internal.h"
-- 
2.25.0.rc1.19.g042ed3e048af


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

* Re: [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump
  2021-01-06 10:09   ` [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump Viresh Kumar
@ 2021-01-06 12:24     ` kernel test robot
  2021-01-06 15:52     ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: kernel test robot @ 2021-01-06 12:24 UTC (permalink / raw)
  To: Viresh Kumar, Pantelis Antoniou, Frank Rowand, Rob Herring
  Cc: kbuild-all, clang-built-linux, Viresh Kumar, devicetree,
	linux-kernel, linux-kbuild, Vincent Guittot, Bill Mills,
	anmar.oueja

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

Hi Viresh,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Viresh-Kumar/scripts-dtc-Start-building-fdtoverlay-and-fdtdump/20210106-181140
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-a013-20210106 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/5ee2a6ba3d6de17f6e9b9dc259081630b6deffb8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Viresh-Kumar/scripts-dtc-Start-building-fdtoverlay-and-fdtdump/20210106-181140
        git checkout 5ee2a6ba3d6de17f6e9b9dc259081630b6deffb8
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> make[2]: *** No rule to make target 'scripts/dtc/fdtoverlay.c', needed by 'scripts/dtc/fdtoverlay.o'.
   In file included from scripts/dtc/fdtdump.c:12:
>> scripts/dtc/libfdt/fdt.h:13:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t magic;                   /* magic word FDT_MAGIC */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:14:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t totalsize;               /* total size of DT block */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:15:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t off_dt_struct;           /* offset to structure */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:16:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t off_dt_strings;          /* offset to strings */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:17:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t off_mem_rsvmap;          /* offset to memory reserve map */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:18:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t version;                 /* format version */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:19:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t last_comp_version;       /* last compatible version */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:22:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t boot_cpuid_phys;         /* Which physical CPU id we're
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:25:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t size_dt_strings;         /* size of the strings block */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:28:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t size_dt_struct;          /* size of the structure block */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
>> scripts/dtc/libfdt/fdt.h:32:2: error: unknown type name 'fdt64_t'; did you mean 'int64_t'?
           fdt64_t address;
           ^~~~~~~
           int64_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: 'int64_t' declared here
   typedef __int64_t int64_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:33:2: error: unknown type name 'fdt64_t'; did you mean 'int64_t'?
           fdt64_t size;
           ^~~~~~~
           int64_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: 'int64_t' declared here
   typedef __int64_t int64_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:37:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t tag;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:42:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t tag;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:43:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t len;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:44:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t nameoff;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
>> scripts/dtc/fdtdump.c:156:28: error: too few arguments to function call, expected 2, have 1
           buf = utilfdt_read(argv[1]);
                 ~~~~~~~~~~~~        ^
   scripts/dtc/util.h:97:7: note: 'utilfdt_read' declared here
   char *utilfdt_read(const char *filename, size_t *len);
         ^
   17 errors generated.
   make[2]: *** [scripts/Makefile.host:112: scripts/dtc/fdtdump.o] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1356: scripts_dtc] Error 2
   make[1]: Target 'modules_prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'modules_prepare' not remade because of errors.
--
>> make[2]: *** No rule to make target 'scripts/dtc/fdtoverlay.c', needed by 'scripts/dtc/fdtoverlay.o'.
   In file included from scripts/dtc/fdtdump.c:12:
>> scripts/dtc/libfdt/fdt.h:13:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t magic;                   /* magic word FDT_MAGIC */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:14:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t totalsize;               /* total size of DT block */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:15:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t off_dt_struct;           /* offset to structure */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:16:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t off_dt_strings;          /* offset to strings */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:17:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t off_mem_rsvmap;          /* offset to memory reserve map */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:18:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t version;                 /* format version */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:19:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t last_comp_version;       /* last compatible version */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:22:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t boot_cpuid_phys;         /* Which physical CPU id we're
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:25:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t size_dt_strings;         /* size of the strings block */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:28:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t size_dt_struct;          /* size of the structure block */
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
>> scripts/dtc/libfdt/fdt.h:32:2: error: unknown type name 'fdt64_t'; did you mean 'int64_t'?
           fdt64_t address;
           ^~~~~~~
           int64_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: 'int64_t' declared here
   typedef __int64_t int64_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:33:2: error: unknown type name 'fdt64_t'; did you mean 'int64_t'?
           fdt64_t size;
           ^~~~~~~
           int64_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:27:19: note: 'int64_t' declared here
   typedef __int64_t int64_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:37:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t tag;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:42:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t tag;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:43:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t len;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
   In file included from scripts/dtc/fdtdump.c:12:
   scripts/dtc/libfdt/fdt.h:44:2: error: unknown type name 'fdt32_t'; did you mean 'int32_t'?
           fdt32_t nameoff;
           ^~~~~~~
           int32_t
   /usr/include/x86_64-linux-gnu/bits/stdint-intn.h:26:19: note: 'int32_t' declared here
   typedef __int32_t int32_t;
                     ^
>> scripts/dtc/fdtdump.c:156:28: error: too few arguments to function call, expected 2, have 1
           buf = utilfdt_read(argv[1]);
                 ~~~~~~~~~~~~        ^
   scripts/dtc/util.h:97:7: note: 'utilfdt_read' declared here
   char *utilfdt_read(const char *filename, size_t *len);
         ^
   17 errors generated.
   make[2]: *** [scripts/Makefile.host:112: scripts/dtc/fdtdump.o] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1356: scripts_dtc] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +13 scripts/dtc/libfdt/fdt.h

1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  11  
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  12  struct fdt_header {
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29 @13  	fdt32_t magic;			 /* magic word FDT_MAGIC */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  14  	fdt32_t totalsize;		 /* total size of DT block */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  15  	fdt32_t off_dt_struct;		 /* offset to structure */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  16  	fdt32_t off_dt_strings;		 /* offset to strings */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  17  	fdt32_t off_mem_rsvmap;		 /* offset to memory reserve map */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  18  	fdt32_t version;		 /* format version */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  19  	fdt32_t last_comp_version;	 /* last compatible version */
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  20  
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  21  	/* version 2 fields below */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  22  	fdt32_t boot_cpuid_phys;	 /* Which physical CPU id we're
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  23  					    booting on */
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  24  	/* version 3 fields below */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  25  	fdt32_t size_dt_strings;	 /* size of the strings block */
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  26  
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  27  	/* version 17 fields below */
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  28  	fdt32_t size_dt_struct;		 /* size of the structure block */
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  29  };
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  30  
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  31  struct fdt_reserve_entry {
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29 @32  	fdt64_t address;
4760597116e34bd scripts/dtc/libfdt/fdt.h       Rob Herring  2015-04-29  33  	fdt64_t size;
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  34  };
1cade99497c881a arch/powerpc/boot/libfdt/fdt.h David Gibson 2007-12-10  35  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35235 bytes --]

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

* Re: [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump
  2021-01-06 10:09   ` [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump Viresh Kumar
  2021-01-06 12:24     ` kernel test robot
@ 2021-01-06 15:52     ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: Rob Herring @ 2021-01-06 15:52 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Pantelis Antoniou, Frank Rowand, devicetree, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	Anmar Oueja

On Wed, Jan 6, 2021 at 3:09 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> We will start building overlays for platforms soon in the kernel and
> would need these tools going forward. Lets fetch and build these.
>
> Note that a copy of fdtdump.c was already copied back in the year 2012,
> but it was never updated or built for some reason.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  scripts/dtc/Makefile             | 8 +++++++-
>  scripts/dtc/update-dtc-source.sh | 6 +++---

This needs to be 2 patches so we can do a dtc sync in between. So
update update-dtc-source.sh, run update-dtc-source.sh, update the
Makefile.

>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index 4852bf44e913..c607980a5c17 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -1,12 +1,18 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # scripts/dtc makefile
>
> -hostprogs-always-$(CONFIG_DTC)         += dtc
> +hostprogs-always-$(CONFIG_DTC)         += dtc fdtdump fdtoverlay
>  hostprogs-always-$(CHECK_DT_BINDING)   += dtc
>
>  dtc-objs       := dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
>                    srcpos.o checks.o util.o
>  dtc-objs       += dtc-lexer.lex.o dtc-parser.tab.o
> +fdtdump-objs   := fdtdump.o util.o
> +
> +libfdt_dir     = libfdt
> +libfdt-objs    := fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o
> +libfdt         = $(addprefix $(libfdt_dir)/,$(libfdt-objs))
> +fdtoverlay-objs        := $(libfdt) fdtoverlay.o util.o
>
>  # Source files need to get at the userspace version of libfdt_env.h to compile
>  HOST_EXTRACFLAGS += -I $(srctree)/$(src)/libfdt
> diff --git a/scripts/dtc/update-dtc-source.sh b/scripts/dtc/update-dtc-source.sh
> index bc704e2a6a4a..9bc4afb71415 100755
> --- a/scripts/dtc/update-dtc-source.sh
> +++ b/scripts/dtc/update-dtc-source.sh
> @@ -31,9 +31,9 @@ set -ev
>  DTC_UPSTREAM_PATH=`pwd`/../dtc
>  DTC_LINUX_PATH=`pwd`/scripts/dtc
>
> -DTC_SOURCE="checks.c data.c dtc.c dtc.h flattree.c fstree.c livetree.c srcpos.c \
> -               srcpos.h treesource.c util.c util.h version_gen.h yamltree.c \
> -               dtc-lexer.l dtc-parser.y"
> +DTC_SOURCE="checks.c data.c dtc.c dtc.h fdtdump.c fdtoverlay.c flattree.c \
> +               fstree.c livetree.c srcpos.c srcpos.h treesource.c util.c \
> +               util.h version_gen.h yamltree.c dtc-lexer.l dtc-parser.y"
>  LIBFDT_SOURCE="fdt.c fdt.h fdt_addresses.c fdt_empty_tree.c \
>                 fdt_overlay.c fdt_ro.c fdt_rw.c fdt_strerror.c fdt_sw.c \
>                 fdt_wip.c libfdt.h libfdt_env.h libfdt_internal.h"
> --
> 2.25.0.rc1.19.g042ed3e048af
>

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-05 15:21 ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Rob Herring
  2021-01-06 10:09   ` [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump Viresh Kumar
@ 2021-01-07  5:28   ` Masahiro Yamada
  2021-01-07  7:27     ` Viresh Kumar
                       ` (2 more replies)
  2021-01-12  0:25   ` Frank Rowand
  2 siblings, 3 replies; 20+ messages in thread
From: Masahiro Yamada @ 2021-01-07  5:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: Viresh Kumar, Arnd Bergmann, Olof Johansson, Pantelis Antoniou,
	Frank Rowand, Michal Marek, DTML, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	tero.kristo

On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > Hello,
> >
> > Here is an attempt to make some changes in the kernel to allow building
> > of device tree overlays.
> >
> > While at it, I would also like to discuss about how we should mention
> > the base DT blobs in the Makefiles for the overlays, so they can be
> > build tested to make sure the overlays apply properly.
> >
> > A simple way is to mention that with -base extension, like this:
> >
> > $(overlay-file)-base := platform-base.dtb
> >
> > Any other preference ?



Viresh's patch is not enough.

We will need to change .gitignore
and scripts/Makefile.dtbinst as well.


In my understanding, the build rule is completely the same
between .dtb and .dtbo
As Rob mentioned, I am not sure if we really need/want
a separate extension.


A counter approach is to use an extension like '.ovl.dtb'
It clarifies it is an overlay fragment without changing
anything in our build system or the upstream DTC project.

We use chained extension in some places, for example,
.dt.yaml for schema yaml files.



dtb-$(CONFIG_ARCH_FOO) += \
    foo-board.dtb \
    foo-overlay1.ovl.dtb \
    foo-overlay2.ovl.dtb


Overlay DT source file names must end with '.ovl.dts'




>
> I think we'll want something similar to how '-objs' works for modules:
>
> foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo
> foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo
> foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo
> dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb
>
> (One difference here is we will want all the intermediate targets
> unlike .o files.)
>
> You wouldn't necessarily have all the above combinations, but you have
> to allow for them. I'm not sure how we'd handle applying any common
> overlays where the base and overlay are in different directories.


I guess the motivation for supporting -dtbs is to
add per-board -@ option only when it contains *.dtbo pattern.

But, as you notice, if the overlay files are located
under drivers/, it is difficult to add -@ per board.

Another scenario is, some people may want to compile
downstream overlay files (i.e. similar concept as external modules),
then we have no idea which base board should be given with the -@ flag.


I'd rather be tempted to add it globally


ifdef CONFIG_OF_OVERLAY
DTC_FLAGS += -@
endif







>
> Another thing here is adding all the above is not really going to
> scale on arm32 where we have a single dts directory. We need to move
> things to per vendor/soc family directories. I have the script to do
> this. We just need to agree on the vendor names and get Arnd/Olof to
> run it. I also want that so we can enable schema checks by default
> once a vendor is warning free (the whole tree is going to take
> forever).


If this is a big churn, perhaps we could make it extreme
to decouple DT and Linux-arch.



arch/*/boot/dts/*.dts
 ->  dts/<vendor>/*.dts

Documentation/devicetree/bindings
 -> dts/Bindings/

include/dt-bindings/
 -> dts/include/dt-bindings/



Then, other project can take dts/
to reuse for them.







> > Also fdtoverlay is an external entity right now, and is not part of the
> > kernel. Do we need to make it part of the kernel ? Or keep using the
> > external entity ?
>
> Part of the kernel. We just need to add it to the dtc sync script and
> makefile I think.
>
> Rob



--
Best Regards
Masahiro Yamada

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-07  5:28   ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Masahiro Yamada
@ 2021-01-07  7:27     ` Viresh Kumar
  2021-01-07 19:02     ` Rob Herring
  2021-01-11 11:17     ` Viresh Kumar
  2 siblings, 0 replies; 20+ messages in thread
From: Viresh Kumar @ 2021-01-07  7:27 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Rob Herring, Arnd Bergmann, Olof Johansson, Pantelis Antoniou,
	Frank Rowand, Michal Marek, DTML, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	tero.kristo

On 07-01-21, 14:28, Masahiro Yamada wrote:
> On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > Hello,
> > >
> > > Here is an attempt to make some changes in the kernel to allow building
> > > of device tree overlays.
> > >
> > > While at it, I would also like to discuss about how we should mention
> > > the base DT blobs in the Makefiles for the overlays, so they can be
> > > build tested to make sure the overlays apply properly.
> > >
> > > A simple way is to mention that with -base extension, like this:
> > >
> > > $(overlay-file)-base := platform-base.dtb
> > >
> > > Any other preference ?
> 
> Viresh's patch is not enough.
> 
> We will need to change .gitignore
> and scripts/Makefile.dtbinst as well.

Thanks.
 
> In my understanding, the build rule is completely the same
> between .dtb and .dtbo

Right.

> As Rob mentioned, I am not sure if we really need/want
> a separate extension.
> 
> 
> A counter approach is to use an extension like '.ovl.dtb'
> It clarifies it is an overlay fragment without changing
> anything in our build system or the upstream DTC project.
> 
> We use chained extension in some places, for example,
> .dt.yaml for schema yaml files.
> 
> 
> 
> dtb-$(CONFIG_ARCH_FOO) += \
>     foo-board.dtb \
>     foo-overlay1.ovl.dtb \
>     foo-overlay2.ovl.dtb
> 
> 
> Overlay DT source file names must end with '.ovl.dts'

I am fine with any approach that you guys feel is better, .dts or .ovl.dts. I
wanted to start a discussion where we can resolve this and be done with it.

Thanks.

-- 
viresh

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-07  5:28   ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Masahiro Yamada
  2021-01-07  7:27     ` Viresh Kumar
@ 2021-01-07 19:02     ` Rob Herring
  2021-01-07 19:42       ` Bill Mills
  2021-01-11 17:26       ` Masahiro Yamada
  2021-01-11 11:17     ` Viresh Kumar
  2 siblings, 2 replies; 20+ messages in thread
From: Rob Herring @ 2021-01-07 19:02 UTC (permalink / raw)
  To: Masahiro Yamada, Viresh Kumar
  Cc: Arnd Bergmann, Olof Johansson, Pantelis Antoniou, Frank Rowand,
	Michal Marek, DTML, linux-kernel, Linux Kbuild mailing list,
	Vincent Guittot, Bill Mills, tero.kristo

On Wed, Jan 6, 2021 at 10:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > Hello,
> > >
> > > Here is an attempt to make some changes in the kernel to allow building
> > > of device tree overlays.
> > >
> > > While at it, I would also like to discuss about how we should mention
> > > the base DT blobs in the Makefiles for the overlays, so they can be
> > > build tested to make sure the overlays apply properly.
> > >
> > > A simple way is to mention that with -base extension, like this:
> > >
> > > $(overlay-file)-base := platform-base.dtb
> > >
> > > Any other preference ?
>
>
>
> Viresh's patch is not enough.
>
> We will need to change .gitignore
> and scripts/Makefile.dtbinst as well.
>
>
> In my understanding, the build rule is completely the same
> between .dtb and .dtbo
> As Rob mentioned, I am not sure if we really need/want
> a separate extension.
>
>
> A counter approach is to use an extension like '.ovl.dtb'
> It clarifies it is an overlay fragment without changing
> anything in our build system or the upstream DTC project.
>
> We use chained extension in some places, for example,
> .dt.yaml for schema yaml files.
>
>
>
> dtb-$(CONFIG_ARCH_FOO) += \
>     foo-board.dtb \
>     foo-overlay1.ovl.dtb \
>     foo-overlay2.ovl.dtb
>
>
> Overlay DT source file names must end with '.ovl.dts'

I like that suggestion as then it's also clear looking at the source
files which ones are overlays. Or we'd need .dtso to be consistent.


> > I think we'll want something similar to how '-objs' works for modules:
> >
> > foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo
> > foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo
> > foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo
> > dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb
> >
> > (One difference here is we will want all the intermediate targets
> > unlike .o files.)
> >
> > You wouldn't necessarily have all the above combinations, but you have
> > to allow for them. I'm not sure how we'd handle applying any common
> > overlays where the base and overlay are in different directories.
>
>
> I guess the motivation for supporting -dtbs is to
> add per-board -@ option only when it contains *.dtbo pattern.

I hadn't thought that far, but yeah, that would be good. Really, I
just want it to be controlled per SoC family at least.

> But, as you notice, if the overlay files are located
> under drivers/, it is difficult to add -@ per board.

Generally, they shouldn't be. The exceptions are what we already have
there which are old dt fixups and unittests.

We want the stripped DT repo (devicetree-rebasing) to have all this
and drivers/ is stripped out. (Which reminds me, the DT repo will need
some work to support all this. It's a different build sys.)

> Another scenario is, some people may want to compile
> downstream overlay files (i.e. similar concept as external modules),
> then we have no idea which base board should be given with the -@ flag.
>
>
> I'd rather be tempted to add it globally
>
>
> ifdef CONFIG_OF_OVERLAY
> DTC_FLAGS += -@
> endif

We've already rejected doing that. Turning on '-@' can grow the dtb
size by a significant amount which could be problematic for some
boards.


> > Another thing here is adding all the above is not really going to
> > scale on arm32 where we have a single dts directory. We need to move
> > things to per vendor/soc family directories. I have the script to do
> > this. We just need to agree on the vendor names and get Arnd/Olof to
> > run it. I also want that so we can enable schema checks by default
> > once a vendor is warning free (the whole tree is going to take
> > forever).
>
>
> If this is a big churn, perhaps we could make it extreme
> to decouple DT and Linux-arch.

I would be fine with that, but I don't think we'll get agreement
there. With that amount of change, we'll be discussing git submodule
again.

Rereading the thread on vendor directories[1], we may just move boards
one vendor at a time. We could just make that a prerequisite for
vendor supporting overlays.

> arch/*/boot/dts/*.dts
>  ->  dts/<vendor>/*.dts
>
> Documentation/devicetree/bindings
>  -> dts/Bindings/
>
> include/dt-bindings/
>  -> dts/include/dt-bindings/
>
>
>
> Then, other project can take dts/
> to reuse for them.

This is already possible with devicetree-rebasing.git. Though it is
still by arch.

Rob

[1] https://lore.kernel.org/linux-devicetree/20181204183649.GA5716@bogus/

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-07 19:02     ` Rob Herring
@ 2021-01-07 19:42       ` Bill Mills
  2021-01-11 17:26       ` Masahiro Yamada
  1 sibling, 0 replies; 20+ messages in thread
From: Bill Mills @ 2021-01-07 19:42 UTC (permalink / raw)
  To: Rob Herring, Masahiro Yamada, Viresh Kumar
  Cc: Arnd Bergmann, Olof Johansson, Pantelis Antoniou, Frank Rowand,
	Michal Marek, DTML, linux-kernel, Linux Kbuild mailing list,
	Vincent Guittot, tero.kristo

Hello,

On 1/7/21 2:02 PM, Rob Herring wrote:
> On Wed, Jan 6, 2021 at 10:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote:
>>>
>>> On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>>
>>>> Hello,
>>>>
>>>> Here is an attempt to make some changes in the kernel to allow building
>>>> of device tree overlays.
>>>>
>>>> While at it, I would also like to discuss about how we should mention
>>>> the base DT blobs in the Makefiles for the overlays, so they can be
>>>> build tested to make sure the overlays apply properly.
>>>>
>>>> A simple way is to mention that with -base extension, like this:
>>>>
>>>> $(overlay-file)-base := platform-base.dtb
>>>>
>>>> Any other preference ?
>>
>>
>>
>> Viresh's patch is not enough.
>>
>> We will need to change .gitignore
>> and scripts/Makefile.dtbinst as well.
>>
>>
>> In my understanding, the build rule is completely the same
>> between .dtb and .dtbo
>> As Rob mentioned, I am not sure if we really need/want
>> a separate extension.
>>
>>
>> A counter approach is to use an extension like '.ovl.dtb'
>> It clarifies it is an overlay fragment without changing
>> anything in our build system or the upstream DTC project.

*.dtbo is already a well established defato standard.  I see little 
value in changing it and doing so will likely just bifurcate common usage.

>>
>> We use chained extension in some places, for example,
>> .dt.yaml for schema yaml files.
>>
>>
>>
>> dtb-$(CONFIG_ARCH_FOO) += \
>>      foo-board.dtb \
>>      foo-overlay1.ovl.dtb \
>>      foo-overlay2.ovl.dtb
>>
>>
>> Overlay DT source file names must end with '.ovl.dts'
> 
> I like that suggestion as then it's also clear looking at the source
> files which ones are overlays. Or we'd need .dtso to be consistent.
> 

I don't think there is much of a problem renaming the source side.
Don't know if that helps if the output is still dtbo.

"Be consistent on dtso" sounds good to me.
Can it be enforced via build time checks?

> 
>>> I think we'll want something similar to how '-objs' works for modules:
>>>
>>> foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo
>>> foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo
>>> foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo
>>> dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb
>>>
>>> (One difference here is we will want all the intermediate targets
>>> unlike .o files.)
>>>
>>> You wouldn't necessarily have all the above combinations, but you have
>>> to allow for them. I'm not sure how we'd handle applying any common
>>> overlays where the base and overlay are in different directories.
>>
>>
>> I guess the motivation for supporting -dtbs is to
>> add per-board -@ option only when it contains *.dtbo pattern.
> 
> I hadn't thought that far, but yeah, that would be good. Really, I
> just want it to be controlled per SoC family at least.
> 
>> But, as you notice, if the overlay files are located
>> under drivers/, it is difficult to add -@ per board.
> 
> Generally, they shouldn't be. The exceptions are what we already have
> there which are old dt fixups and unittests.
> 
> We want the stripped DT repo (devicetree-rebasing) to have all this
> and drivers/ is stripped out. (Which reminds me, the DT repo will need
> some work to support all this. It's a different build sys.)
> 
>> Another scenario is, some people may want to compile
>> downstream overlay files (i.e. similar concept as external modules),
>> then we have no idea which base board should be given with the -@ flag.
>>
>>
>> I'd rather be tempted to add it globally
>>
>>
>> ifdef CONFIG_OF_OVERLAY
>> DTC_FLAGS += -@
>> endif
> 
> We've already rejected doing that. Turning on '-@' can grow the dtb
> size by a significant amount which could be problematic for some
> boards >
>>> Another thing here is adding all the above is not really going to
>>> scale on arm32 where we have a single dts directory. We need to move
>>> things to per vendor/soc family directories. I have the script to do
>>> this. We just need to agree on the vendor names and get Arnd/Olof to
>>> run it. I also want that so we can enable schema checks by default
>>> once a vendor is warning free (the whole tree is going to take
>>> forever).
>>
>>
>> If this is a big churn, perhaps we could make it extreme
>> to decouple DT and Linux-arch.
> 
> I would be fine with that, but I don't think we'll get agreement
> there. With that amount of change, we'll be discussing git submodule
> again.
> 
> Rereading the thread on vendor directories[1], we may just move boards
> one vendor at a time. We could just make that a prerequisite for
> vendor supporting overlays.
> 
>> arch/*/boot/dts/*.dts
>>   ->  dts/<vendor>/*.dts
>>
>> Documentation/devicetree/bindings
>>   -> dts/Bindings/
>>
>> include/dt-bindings/
>>   -> dts/include/dt-bindings/
>>
>>
>>
>> Then, other project can take dts/
>> to reuse for them.
> 
> This is already possible with devicetree-rebasing.git. Though it is
> still by arch.
> 
> Rob
> 
> [1] https://lore.kernel.org/linux-devicetree/20181204183649.GA5716@bogus/
> 

Thanks for digging up this script!

Bill

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-07  5:28   ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Masahiro Yamada
  2021-01-07  7:27     ` Viresh Kumar
  2021-01-07 19:02     ` Rob Herring
@ 2021-01-11 11:17     ` Viresh Kumar
  2021-01-11 15:40       ` Masahiro Yamada
  2 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2021-01-11 11:17 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Rob Herring, Arnd Bergmann, Olof Johansson, Pantelis Antoniou,
	Frank Rowand, Michal Marek, DTML, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	tero.kristo

On 07-01-21, 14:28, Masahiro Yamada wrote:
> Viresh's patch is not enough.
> 
> We will need to change .gitignore
> and scripts/Makefile.dtbinst as well.
> 
> In my understanding, the build rule is completely the same
> between .dtb and .dtbo
> As Rob mentioned, I am not sure if we really need/want
> a separate extension.
> 
> A counter approach is to use an extension like '.ovl.dtb'
> It clarifies it is an overlay fragment without changing
> anything in our build system or the upstream DTC project.

By the time you gave feedback, I have already sent the dtbo change for
DTC to the device-tree-compiler list (based on Rob's suggestion).

And it got merged today by David:

https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc

Can we please finalize what we need to do with naming here and be done
with it, so I can rework my patches and get going ?

Thanks.

-- 
viresh

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-11 11:17     ` Viresh Kumar
@ 2021-01-11 15:40       ` Masahiro Yamada
  2021-01-11 16:13         ` Rob Herring
  0 siblings, 1 reply; 20+ messages in thread
From: Masahiro Yamada @ 2021-01-11 15:40 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rob Herring, Arnd Bergmann, Olof Johansson, Pantelis Antoniou,
	Frank Rowand, Michal Marek, DTML, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	tero.kristo

On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 07-01-21, 14:28, Masahiro Yamada wrote:
> > Viresh's patch is not enough.
> >
> > We will need to change .gitignore
> > and scripts/Makefile.dtbinst as well.
> >
> > In my understanding, the build rule is completely the same
> > between .dtb and .dtbo
> > As Rob mentioned, I am not sure if we really need/want
> > a separate extension.
> >
> > A counter approach is to use an extension like '.ovl.dtb'
> > It clarifies it is an overlay fragment without changing
> > anything in our build system or the upstream DTC project.
>
> By the time you gave feedback, I have already sent the dtbo change for
> DTC to the device-tree-compiler list (based on Rob's suggestion).
>
> And it got merged today by David:
>
> https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc
>
> Can we please finalize what we need to do with naming here and be done
> with it, so I can rework my patches and get going ?
>
> Thanks.
>
> --
> viresh



It is unfortunate to see such a patch merged
before getting agreement about how it should work
as a whole.




>+# enable creation of __symbols__ node
>+ifneq ($(dtbo-y),)
>+DTC_FLAGS += -@
>+endif

I am not convinced with this code.

A single user of the dtbo-y syntax gives -@ to all
device trees in the same directory.

This is not a solution since Rob already stated -@ should be
given per board (or per platform, at least).

I still do not understand why adding the new syntax dtbo-y
is helpful.




Have we already decided to use separate ".dtb" and ".dtbo" for blobs?

Will we use ".dts" for all source files?
Or, will we use ".dtso" for overlay source files?

How should the build system determine the targets
that should have -@ option?



For consistency, will we need a patch like follows?


diff --git a/dtc.c b/dtc.c
index bdb3f59..474401e 100644
--- a/dtc.c
+++ b/dtc.c
@@ -120,6 +120,8 @@ static const char *guess_type_by_name(const char
*fname, const char *fallback)
                return fallback;
        if (!strcasecmp(s, ".dts"))
                return "dts";
+       if (!strcasecmp(s, ".dtso"))
+               return "dts";
        if (!strcasecmp(s, ".yaml"))
                return "yaml";
        if (!strcasecmp(s, ".dtb"))
@@ -349,6 +351,8 @@ int main(int argc, char *argv[])

        if (streq(outform, "dts")) {
                dt_to_source(outf, dti);
+       else if (streq(outform, "dtso")) {
+               dt_to_source(outf, dti);
 #ifndef NO_YAML
        } else if (streq(outform, "yaml")) {
                if (!streq(inform, "dts"))



Overall solution looks unclear to me.


Again, it is unfortunate that we did not take enough time
(in spite of the RFC prefix) before proceeding.


-- 
Best Regards
Masahiro Yamada

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-11 15:40       ` Masahiro Yamada
@ 2021-01-11 16:13         ` Rob Herring
  2021-01-11 17:02           ` Masahiro Yamada
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-01-11 16:13 UTC (permalink / raw)
  To: Masahiro Yamada, David Gibson, Viresh Kumar
  Cc: Arnd Bergmann, Olof Johansson, Pantelis Antoniou, Frank Rowand,
	Michal Marek, DTML, linux-kernel, Linux Kbuild mailing list,
	Vincent Guittot, Bill Mills, tero.kristo

+David Gibson

On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 07-01-21, 14:28, Masahiro Yamada wrote:
> > > Viresh's patch is not enough.
> > >
> > > We will need to change .gitignore
> > > and scripts/Makefile.dtbinst as well.
> > >
> > > In my understanding, the build rule is completely the same
> > > between .dtb and .dtbo
> > > As Rob mentioned, I am not sure if we really need/want
> > > a separate extension.
> > >
> > > A counter approach is to use an extension like '.ovl.dtb'
> > > It clarifies it is an overlay fragment without changing
> > > anything in our build system or the upstream DTC project.
> >
> > By the time you gave feedback, I have already sent the dtbo change for
> > DTC to the device-tree-compiler list (based on Rob's suggestion).
> >
> > And it got merged today by David:
> >
> > https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc
> >
> > Can we please finalize what we need to do with naming here and be done
> > with it, so I can rework my patches and get going ?
> >
> > Thanks.
> >
> > --
> > viresh
>
>
>
> It is unfortunate to see such a patch merged
> before getting agreement about how it should work
> as a whole.

Given the feedback that dtbo is already a standard, I'd suggest we
just stick with dts->dtbo.

> >+# enable creation of __symbols__ node
> >+ifneq ($(dtbo-y),)
> >+DTC_FLAGS += -@
> >+endif
>
> I am not convinced with this code.
>
> A single user of the dtbo-y syntax gives -@ to all
> device trees in the same directory.
>
> This is not a solution since Rob already stated -@ should be
> given per board (or per platform, at least).

Agreed.

> I still do not understand why adding the new syntax dtbo-y
> is helpful.

I think we should stick with 'dtb-y' here.


> Have we already decided to use separate ".dtb" and ".dtbo" for blobs?
>
> Will we use ".dts" for all source files?
> Or, will we use ".dtso" for overlay source files?
>
> How should the build system determine the targets
> that should have -@ option?

The way it does already. Either:

DTC_FLAGS += -@

in a directory of dts files. Or on a per file basis like:

DTC_FLAGS_foo_base += -@

> For consistency, will we need a patch like follows?
>
>
> diff --git a/dtc.c b/dtc.c
> index bdb3f59..474401e 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -120,6 +120,8 @@ static const char *guess_type_by_name(const char
> *fname, const char *fallback)
>                 return fallback;
>         if (!strcasecmp(s, ".dts"))
>                 return "dts";
> +       if (!strcasecmp(s, ".dtso"))
> +               return "dts";
>         if (!strcasecmp(s, ".yaml"))
>                 return "yaml";
>         if (!strcasecmp(s, ".dtb"))
> @@ -349,6 +351,8 @@ int main(int argc, char *argv[])
>
>         if (streq(outform, "dts")) {
>                 dt_to_source(outf, dti);
> +       else if (streq(outform, "dtso")) {
> +               dt_to_source(outf, dti);
>  #ifndef NO_YAML
>         } else if (streq(outform, "yaml")) {
>                 if (!streq(inform, "dts"))
>
>
>
> Overall solution looks unclear to me.
>
>
> Again, it is unfortunate that we did not take enough time
> (in spite of the RFC prefix) before proceeding.

I should have added David here from the start. Honestly, I expected
some discussion there.

Rob

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-11 16:13         ` Rob Herring
@ 2021-01-11 17:02           ` Masahiro Yamada
  2021-01-12  9:41             ` Viresh Kumar
  0 siblings, 1 reply; 20+ messages in thread
From: Masahiro Yamada @ 2021-01-11 17:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Gibson, Viresh Kumar, Arnd Bergmann, Olof Johansson,
	Pantelis Antoniou, Frank Rowand, Michal Marek, DTML,
	linux-kernel, Linux Kbuild mailing list, Vincent Guittot,
	Bill Mills, tero.kristo

On Tue, Jan 12, 2021 at 1:13 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> +David Gibson
>
> On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Mon, Jan 11, 2021 at 8:17 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > On 07-01-21, 14:28, Masahiro Yamada wrote:
> > > > Viresh's patch is not enough.
> > > >
> > > > We will need to change .gitignore
> > > > and scripts/Makefile.dtbinst as well.
> > > >
> > > > In my understanding, the build rule is completely the same
> > > > between .dtb and .dtbo
> > > > As Rob mentioned, I am not sure if we really need/want
> > > > a separate extension.
> > > >
> > > > A counter approach is to use an extension like '.ovl.dtb'
> > > > It clarifies it is an overlay fragment without changing
> > > > anything in our build system or the upstream DTC project.
> > >
> > > By the time you gave feedback, I have already sent the dtbo change for
> > > DTC to the device-tree-compiler list (based on Rob's suggestion).
> > >
> > > And it got merged today by David:
> > >
> > > https://github.com/dgibson/dtc/commit/163f0469bf2ed8b2fe5aa15bc796b93c70243ddc
> > >
> > > Can we please finalize what we need to do with naming here and be done
> > > with it, so I can rework my patches and get going ?
> > >
> > > Thanks.
> > >
> > > --
> > > viresh
> >
> >
> >
> > It is unfortunate to see such a patch merged
> > before getting agreement about how it should work
> > as a whole.
>
> Given the feedback that dtbo is already a standard, I'd suggest we
> just stick with dts->dtbo.

OK.
dtbo seems a stanrdard already...



> > >+# enable creation of __symbols__ node
> > >+ifneq ($(dtbo-y),)
> > >+DTC_FLAGS += -@
> > >+endif
> >
> > I am not convinced with this code.
> >
> > A single user of the dtbo-y syntax gives -@ to all
> > device trees in the same directory.
> >
> > This is not a solution since Rob already stated -@ should be
> > given per board (or per platform, at least).
>
> Agreed.
>
> > I still do not understand why adding the new syntax dtbo-y
> > is helpful.
>
> I think we should stick with 'dtb-y' here.
>
>
> > Have we already decided to use separate ".dtb" and ".dtbo" for blobs?
> >
> > Will we use ".dts" for all source files?
> > Or, will we use ".dtso" for overlay source files?
> >
> > How should the build system determine the targets
> > that should have -@ option?
>
> The way it does already. Either:
>
> DTC_FLAGS += -@
>
> in a directory of dts files. Or on a per file basis like:
>
> DTC_FLAGS_foo_base += -@


Ah yes.  I like this.




We do not need the dtbo-y syntax.


We can simply use dtb-y for
base boards and overlay fragments:



dtb-$(CONFIG_ARCH_FOO) += \
     foo-base.dtb \
     foo-overlay1.dtbo \
     foo-overlay2.dtbo

DTB_FLAGS_foo-base += -@







>
> > For consistency, will we need a patch like follows?
> >
> >
> > diff --git a/dtc.c b/dtc.c
> > index bdb3f59..474401e 100644
> > --- a/dtc.c
> > +++ b/dtc.c
> > @@ -120,6 +120,8 @@ static const char *guess_type_by_name(const char
> > *fname, const char *fallback)
> >                 return fallback;
> >         if (!strcasecmp(s, ".dts"))
> >                 return "dts";
> > +       if (!strcasecmp(s, ".dtso"))
> > +               return "dts";
> >         if (!strcasecmp(s, ".yaml"))
> >                 return "yaml";
> >         if (!strcasecmp(s, ".dtb"))
> > @@ -349,6 +351,8 @@ int main(int argc, char *argv[])
> >
> >         if (streq(outform, "dts")) {
> >                 dt_to_source(outf, dti);
> > +       else if (streq(outform, "dtso")) {
> > +               dt_to_source(outf, dti);
> >  #ifndef NO_YAML
> >         } else if (streq(outform, "yaml")) {
> >                 if (!streq(inform, "dts"))
> >
> >
> >
> > Overall solution looks unclear to me.
> >
> >
> > Again, it is unfortunate that we did not take enough time
> > (in spite of the RFC prefix) before proceeding.
>
> I should have added David here from the start. Honestly, I expected
> some discussion there.
>
> Rob




-- 
Best Regards
Masahiro Yamada

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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-07 19:02     ` Rob Herring
  2021-01-07 19:42       ` Bill Mills
@ 2021-01-11 17:26       ` Masahiro Yamada
  1 sibling, 0 replies; 20+ messages in thread
From: Masahiro Yamada @ 2021-01-11 17:26 UTC (permalink / raw)
  To: Rob Herring
  Cc: Viresh Kumar, Arnd Bergmann, Olof Johansson, Pantelis Antoniou,
	Frank Rowand, Michal Marek, DTML, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	tero.kristo

On Fri, Jan 8, 2021 at 4:02 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Wed, Jan 6, 2021 at 10:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Jan 6, 2021 at 12:21 AM Rob Herring <robh+dt@kernel.org> wrote:
> > >
> > > On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > >
> > > > Hello,
> > > >
> > > > Here is an attempt to make some changes in the kernel to allow building
> > > > of device tree overlays.
> > > >
> > > > While at it, I would also like to discuss about how we should mention
> > > > the base DT blobs in the Makefiles for the overlays, so they can be
> > > > build tested to make sure the overlays apply properly.
> > > >
> > > > A simple way is to mention that with -base extension, like this:
> > > >
> > > > $(overlay-file)-base := platform-base.dtb
> > > >
> > > > Any other preference ?
> >
> >
> >
> > Viresh's patch is not enough.
> >
> > We will need to change .gitignore
> > and scripts/Makefile.dtbinst as well.
> >
> >
> > In my understanding, the build rule is completely the same
> > between .dtb and .dtbo
> > As Rob mentioned, I am not sure if we really need/want
> > a separate extension.
> >
> >
> > A counter approach is to use an extension like '.ovl.dtb'
> > It clarifies it is an overlay fragment without changing
> > anything in our build system or the upstream DTC project.
> >
> > We use chained extension in some places, for example,
> > .dt.yaml for schema yaml files.
> >
> >
> >
> > dtb-$(CONFIG_ARCH_FOO) += \
> >     foo-board.dtb \
> >     foo-overlay1.ovl.dtb \
> >     foo-overlay2.ovl.dtb
> >
> >
> > Overlay DT source file names must end with '.ovl.dts'
>
> I like that suggestion as then it's also clear looking at the source
> files which ones are overlays. Or we'd need .dtso to be consistent.
>
>
> > > I think we'll want something similar to how '-objs' works for modules:
> > >
> > > foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo
> > > foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo
> > > foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo
> > > dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb
> > >
> > > (One difference here is we will want all the intermediate targets
> > > unlike .o files.)
> > >
> > > You wouldn't necessarily have all the above combinations, but you have
> > > to allow for them. I'm not sure how we'd handle applying any common
> > > overlays where the base and overlay are in different directories.
> >
> >
> > I guess the motivation for supporting -dtbs is to
> > add per-board -@ option only when it contains *.dtbo pattern.
>
> I hadn't thought that far, but yeah, that would be good. Really, I
> just want it to be controlled per SoC family at least.
>
> > But, as you notice, if the overlay files are located
> > under drivers/, it is difficult to add -@ per board.
>
> Generally, they shouldn't be. The exceptions are what we already have
> there which are old dt fixups and unittests.
>
> We want the stripped DT repo (devicetree-rebasing) to have all this
> and drivers/ is stripped out. (Which reminds me, the DT repo will need
> some work to support all this. It's a different build sys.)
>
> > Another scenario is, some people may want to compile
> > downstream overlay files (i.e. similar concept as external modules),
> > then we have no idea which base board should be given with the -@ flag.
> >
> >
> > I'd rather be tempted to add it globally
> >
> >
> > ifdef CONFIG_OF_OVERLAY
> > DTC_FLAGS += -@
> > endif
>
> We've already rejected doing that. Turning on '-@' can grow the dtb
> size by a significant amount which could be problematic for some
> boards.
>
>
> > > Another thing here is adding all the above is not really going to
> > > scale on arm32 where we have a single dts directory. We need to move
> > > things to per vendor/soc family directories. I have the script to do
> > > this. We just need to agree on the vendor names and get Arnd/Olof to
> > > run it. I also want that so we can enable schema checks by default
> > > once a vendor is warning free (the whole tree is going to take
> > > forever).
> >
> >
> > If this is a big churn, perhaps we could make it extreme
> > to decouple DT and Linux-arch.
>
> I would be fine with that, but I don't think we'll get agreement
> there. With that amount of change, we'll be discussing git submodule
> again.
>
> Rereading the thread on vendor directories[1], we may just move boards
> one vendor at a time. We could just make that a prerequisite for
> vendor supporting overlays.
>
> > arch/*/boot/dts/*.dts
> >  ->  dts/<vendor>/*.dts
> >
> > Documentation/devicetree/bindings
> >  -> dts/Bindings/
> >
> > include/dt-bindings/
> >  -> dts/include/dt-bindings/
> >
> >
> >
> > Then, other project can take dts/
> > to reuse for them.
>
> This is already possible with devicetree-rebasing.git. Though it is
> still by arch.


Yes, I know this project.

There are still cross-references between arm and arm64.

Associating DT with Linux-arch is not good
because it is possible to boot the 32-bit kernel (arch/arm/)
on the 64-bit boards (arch/arm64/boot/dts/).






> Rob
>
> [1] https://lore.kernel.org/linux-devicetree/20181204183649.GA5716@bogus/



-- 
Best Regards
Masahiro Yamada

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

* Re: [RFC 2/2] scripts: dtc: Handle outform dtbo
  2021-01-05 15:37   ` Rob Herring
@ 2021-01-12  0:18     ` Frank Rowand
  0 siblings, 0 replies; 20+ messages in thread
From: Frank Rowand @ 2021-01-12  0:18 UTC (permalink / raw)
  To: Rob Herring, Viresh Kumar
  Cc: Pantelis Antoniou, devicetree, linux-kernel,
	Linux Kbuild mailing list, Vincent Guittot, Bill Mills,
	tero.kristo

On 1/5/21 9:37 AM, Rob Herring wrote:
> On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>
>> Update dtc compiler to accept dtbo as an outform.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>
>> ---
>> I feel that this needs to go directly to
>> https://git.kernel.org/pub/scm/utils/dtc/dtc.git
>>
>> Right ? I will send it separately if the idea is accepted here.
> 
> Yes, needs to go to devicetree-compiler list. I think this came up
> before and IIRC David wasn't completely in agreement. I looked briefly
> and couldn't find the thread though...
> 
> We really don't need a different extension because we could just
> examine the dtb to determine if it is an overlay or not. That's less
> obvious. We could also add meta-data to overlays defining what base
> they apply to. If we had that, a tool could just list all overlays

It may be valid to apply an overlay may be valid to more than one base FDT.

And for connector nodes and plugin overlays (which do not exist yet, I'm
way behind on bringing that concept forward), a single overlay may be
applied to more than one connector node in the base FDT.

> that should apply to a base and we could use that info for build time
> applying overlays. Of course, that and a dtbo extension/format are not
> mutually exclusive.
> 
>> ---
>>  scripts/dtc/dtc.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
>> index bdb3f5945699..40fa7128b3d6 100644
>> --- a/scripts/dtc/dtc.c
>> +++ b/scripts/dtc/dtc.c
>> @@ -357,6 +357,8 @@ 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")) {
> 
> You also need to extend guess_type_by_name().
> 
> 
> Rob
> 


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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-05 15:21 ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Rob Herring
  2021-01-06 10:09   ` [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump Viresh Kumar
  2021-01-07  5:28   ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Masahiro Yamada
@ 2021-01-12  0:25   ` Frank Rowand
  2 siblings, 0 replies; 20+ messages in thread
From: Frank Rowand @ 2021-01-12  0:25 UTC (permalink / raw)
  To: Rob Herring, Viresh Kumar, Arnd Bergmann, Olof Johansson
  Cc: Pantelis Antoniou, Masahiro Yamada, Michal Marek, devicetree,
	linux-kernel, Linux Kbuild mailing list, Vincent Guittot,
	Bill Mills, tero.kristo

On 1/5/21 9:21 AM, Rob Herring wrote:
> On Tue, Jan 5, 2021 at 4:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>
>> Hello,
>>
>> Here is an attempt to make some changes in the kernel to allow building
>> of device tree overlays.
>>
>> While at it, I would also like to discuss about how we should mention
>> the base DT blobs in the Makefiles for the overlays, so they can be
>> build tested to make sure the overlays apply properly.
>>
>> A simple way is to mention that with -base extension, like this:
>>
>> $(overlay-file)-base := platform-base.dtb
>>
>> Any other preference ?
> 
> I think we'll want something similar to how '-objs' works for modules:
> 
> foo-board-1-dtbs := foo-board.dtb foo-overlay1.dtbo
> foo-board-2-dtbs := foo-board.dtb foo-overlay2.dtbo
> foo-board-1-2-dtbs := foo-board.dtb foo-overlay1.dtbo foo-overlay2.dtbo
> dtbs-y += foo-board-1.dtb foo-board-2.dtb foo-board-1-2.dtb

(Thinking ahead....) I'm not sure how to fit connector nodes and the
corresponding plugin overlays into this model.  A single plugin .dtbo
will need to be relocated onto one or more connector nodes.  fdtoverlay
and the run time overlay apply code do not know how to do this yet.

-Frank

> 
> (One difference here is we will want all the intermediate targets
> unlike .o files.)
> 
> You wouldn't necessarily have all the above combinations, but you have
> to allow for them. I'm not sure how we'd handle applying any common
> overlays where the base and overlay are in different directories.
> 
> Another thing here is adding all the above is not really going to
> scale on arm32 where we have a single dts directory. We need to move
> things to per vendor/soc family directories. I have the script to do
> this. We just need to agree on the vendor names and get Arnd/Olof to
> run it. I also want that so we can enable schema checks by default
> once a vendor is warning free (the whole tree is going to take
> forever).
> 
>> Also fdtoverlay is an external entity right now, and is not part of the
>> kernel. Do we need to make it part of the kernel ? Or keep using the
>> external entity ?
> 
> Part of the kernel. We just need to add it to the dtc sync script and
> makefile I think.
> 
> Rob
> 


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

* Re: [RFC 0/2] kbuild: Add support to build overlays (%.dtbo)
  2021-01-11 17:02           ` Masahiro Yamada
@ 2021-01-12  9:41             ` Viresh Kumar
  0 siblings, 0 replies; 20+ messages in thread
From: Viresh Kumar @ 2021-01-12  9:41 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Rob Herring, David Gibson, Arnd Bergmann, Olof Johansson,
	Pantelis Antoniou, Frank Rowand, Michal Marek, DTML,
	linux-kernel, Linux Kbuild mailing list, Vincent Guittot,
	Bill Mills, tero.kristo

On 12-01-21, 02:02, Masahiro Yamada wrote:
> On Tue, Jan 12, 2021 at 1:13 AM Rob Herring <robh+dt@kernel.org> wrote:
> > On Mon, Jan 11, 2021 at 9:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> We do not need the dtbo-y syntax.

+1

And we are left with much simpler diff with what we agreed on. Does
this look okay now ?

---
 .gitignore               | 3 +--
 Makefile                 | 4 ++--
 scripts/Makefile.dtbinst | 3 +++
 scripts/Makefile.lib     | 4 +++-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/.gitignore b/.gitignore
index d01cda8e1177..0458c36f3cb2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,8 +17,7 @@
 *.bz2
 *.c.[012]*.*
 *.dt.yaml
-*.dtb
-*.dtb.S
+*.dtb*
 *.dwo
 *.elf
 *.gcno
diff --git a/Makefile b/Makefile
index 9e73f82e0d86..b84f5e0b46ab 100644
--- a/Makefile
+++ b/Makefile
@@ -1334,7 +1334,7 @@ endif
 
 ifneq ($(dtstree),)
 
-%.dtb: include/config/kernel.release scripts_dtc
+%.dtb %.dtbo: include/config/kernel.release scripts_dtc
 	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
 
 PHONY += dtbs dtbs_install dtbs_check
@@ -1816,7 +1816,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..30bc0a8e0087 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
@@ -324,7 +326,7 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile)
 
-$(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
+$(obj)/%.dtb $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
 	$(call if_changed_dep,dtc)
 
 DT_CHECKER ?= dt-validate

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

end of thread, other threads:[~2021-01-12  9:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 11:24 [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Viresh Kumar
2021-01-05 11:24 ` [RFC 1/2] " Viresh Kumar
2021-01-05 11:24 ` [RFC 2/2] scripts: dtc: Handle outform dtbo Viresh Kumar
2021-01-05 15:37   ` Rob Herring
2021-01-12  0:18     ` Frank Rowand
2021-01-05 15:21 ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Rob Herring
2021-01-06 10:09   ` [PATCH] scripts: dtc: Start building fdtoverlay and fdtdump Viresh Kumar
2021-01-06 12:24     ` kernel test robot
2021-01-06 15:52     ` Rob Herring
2021-01-07  5:28   ` [RFC 0/2] kbuild: Add support to build overlays (%.dtbo) Masahiro Yamada
2021-01-07  7:27     ` Viresh Kumar
2021-01-07 19:02     ` Rob Herring
2021-01-07 19:42       ` Bill Mills
2021-01-11 17:26       ` Masahiro Yamada
2021-01-11 11:17     ` Viresh Kumar
2021-01-11 15:40       ` Masahiro Yamada
2021-01-11 16:13         ` Rob Herring
2021-01-11 17:02           ` Masahiro Yamada
2021-01-12  9:41             ` Viresh Kumar
2021-01-12  0:25   ` Frank Rowand

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