Message ID | 20231228115804.3626579-4-sumit.garg@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | An effort to bring DT bindings compliance within U-Boot | expand |
Hi Sumit! On December 28, 2023 thus sayeth Sumit Garg: > Allow u-boot to build DTB from a different directory tree such that > *-u-boot.dtsi files can be included from a common location. Currently > that location is arch/$(ARCH)/dts/, so statically define that common > location. > > This is needed for platform owners to start building DTB files from > devicetree-rebasing directory but still being able to include > *-u-boot.dtsi files. > > Reviewed-by: Tom Rini <trini@konsulko.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > --- > ... > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 27b9437027c..09330421856 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib ... > # Uncomment for debugging > @@ -190,6 +192,7 @@ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES) > dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \ > $(UBOOTINCLUDE) \ > -I$(dir $<) \ > + -I$(u_boot_dtsi_loc) \ > -I$(srctree)/arch/$(ARCH)/dts/include \ > -I$(srctree)/include \ > -D__ASSEMBLY__ \ > @@ -328,7 +331,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > $(DTC) -O dtb -o $@ -b 0 \ > - -i $(dir $<) $(DTC_FLAGS) \ > + -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > -d $(depfile).dtc.tmp $(dtc-tmp) || \ > (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \ > ; \ One of the issues I see with having a separate OF_UPSTREAM and U-Boot dt directory is when we have U-Boot board files that use dtsi files in the OF_UPSTREAM folder. For example our reference boards uses the primary bootloader's dtb (eg: k3-am62a7-r5-sk.dts) which #includes the k3-am62a7-sk.dts that will be found in the OF_UPSTREAM directory and modifies it to give it the perspective of the micro-controller it will be running on during boot. What do you think if we have both paths included regardless if OF_UPSTREAM is selected or not? IDK if this will break anyone else ~Bryan
Hi Bryan, On Sat, 6 Jan 2024 at 02:12, Bryan Brattlof <bb@ti.com> wrote: > > Hi Sumit! > > On December 28, 2023 thus sayeth Sumit Garg: > > Allow u-boot to build DTB from a different directory tree such that > > *-u-boot.dtsi files can be included from a common location. Currently > > that location is arch/$(ARCH)/dts/, so statically define that common > > location. > > > > This is needed for platform owners to start building DTB files from > > devicetree-rebasing directory but still being able to include > > *-u-boot.dtsi files. > > > > Reviewed-by: Tom Rini <trini@konsulko.com> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > --- > > > > ... > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index 27b9437027c..09330421856 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > ... > > > # Uncomment for debugging > > @@ -190,6 +192,7 @@ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES) > > dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \ > > $(UBOOTINCLUDE) \ > > -I$(dir $<) \ > > + -I$(u_boot_dtsi_loc) \ > > -I$(srctree)/arch/$(ARCH)/dts/include \ > > -I$(srctree)/include \ > > -D__ASSEMBLY__ \ > > @@ -328,7 +331,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > > $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > > $(DTC) -O dtb -o $@ -b 0 \ > > - -i $(dir $<) $(DTC_FLAGS) \ > > + -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > > -d $(depfile).dtc.tmp $(dtc-tmp) || \ > > (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \ > > ; \ > > One of the issues I see with having a separate OF_UPSTREAM and U-Boot dt > directory is when we have U-Boot board files that use dtsi files in the > OF_UPSTREAM folder. > > For example our reference boards uses the primary bootloader's dtb (eg: > k3-am62a7-r5-sk.dts) which #includes the k3-am62a7-sk.dts that will be > found in the OF_UPSTREAM directory and modifies it to give it the > perspective of the micro-controller it will be running on during boot. Thanks for bringing this up. I have been playing with the idea to reuse DT includes from upstream. > > What do you think if we have both paths included regardless if > OF_UPSTREAM is selected or not? IDK if this will break anyone else Sure, we should be able to do that if we maintain the correct order of include paths as per following patch [1]. If this works for you let me know and I will include it for v4. [1] commit 7fcebc044c69c57b43ff0e59f2d8c3713bc68b6c Author: Sumit Garg <sumit.garg@linaro.org> Date: Tue Jan 2 19:20:01 2024 +0530 Makefile: Allow upstream DT subtree to provide DT includes Allow platforms to reuse DT headers and dtsi includes directly form upstream DT subtree which will be frequently synced with Linux kernel. This will further allow us to drop corresponding DT includes copy from U-Boot tree. Also, since the DT includes from upstream DT subtree are done after DT includes from U-Boot tree, so it shouldn't cause any conflicts. Signed-off-by: Sumit Garg <sumit.garg@linaro.org> diff --git a/Makefile b/Makefile index d8d168cd4c3..0160e827a29 100644 --- a/Makefile +++ b/Makefile @@ -834,7 +834,8 @@ UBOOTINCLUDE := \ -I$(srctree)/arch/arm/thumb1/include), \ -I$(srctree)/arch/arm/thumb1/include)) \ -I$(srctree)/arch/$(ARCH)/include \ - -include $(srctree)/include/linux/kconfig.h + -include $(srctree)/include/linux/kconfig.h \ + -I$(srctree)/devicetree-rebasing/include NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 09330421856..30033092597 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -189,12 +189,17 @@ dtsi_include_list = $(strip $(u_boot_dtsi_options_debug) \ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES) # Modified for U-Boot +upstream_dtsi_include = $(addprefix -I, $(srctree)/devicetree-rebasing/src/ \ + $(sort $(dir $(wildcard $(srctree)/devicetree-rebasing/src/$(ARCH)/*/*))) \ + $(if (CONFIG_ARM64), \ + $(sort $(dir $(wildcard $(srctree)/devicetree-rebasing/src/arm64/*/*))))) dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \ $(UBOOTINCLUDE) \ -I$(dir $<) \ -I$(u_boot_dtsi_loc) \ -I$(srctree)/arch/$(ARCH)/dts/include \ -I$(srctree)/include \ + $(upstream_dtsi_include) \ -D__ASSEMBLY__ \ -undef -D__DTS__ -Sumit
On January 8, 2024 thus sayeth Sumit Garg: > Hi Bryan, > > On Sat, 6 Jan 2024 at 02:12, Bryan Brattlof <bb@ti.com> wrote: > > > > Hi Sumit! > > > > On December 28, 2023 thus sayeth Sumit Garg: > > > Allow u-boot to build DTB from a different directory tree such that > > > *-u-boot.dtsi files can be included from a common location. Currently > > > that location is arch/$(ARCH)/dts/, so statically define that common > > > location. > > > > > > This is needed for platform owners to start building DTB files from > > > devicetree-rebasing directory but still being able to include > > > *-u-boot.dtsi files. > > > > > > Reviewed-by: Tom Rini <trini@konsulko.com> > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > > --- > > > > > > > ... > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > index 27b9437027c..09330421856 100644 > > > --- a/scripts/Makefile.lib > > > +++ b/scripts/Makefile.lib > > > > ... > > > > > # Uncomment for debugging > > > @@ -190,6 +192,7 @@ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES) > > > dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \ > > > $(UBOOTINCLUDE) \ > > > -I$(dir $<) \ > > > + -I$(u_boot_dtsi_loc) \ > > > -I$(srctree)/arch/$(ARCH)/dts/include \ > > > -I$(srctree)/include \ > > > -D__ASSEMBLY__ \ > > > @@ -328,7 +331,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > > echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > > > $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > > > $(DTC) -O dtb -o $@ -b 0 \ > > > - -i $(dir $<) $(DTC_FLAGS) \ > > > + -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > > > -d $(depfile).dtc.tmp $(dtc-tmp) || \ > > > (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \ > > > ; \ > > > > One of the issues I see with having a separate OF_UPSTREAM and U-Boot dt > > directory is when we have U-Boot board files that use dtsi files in the > > OF_UPSTREAM folder. > > > > For example our reference boards uses the primary bootloader's dtb (eg: > > k3-am62a7-r5-sk.dts) which #includes the k3-am62a7-sk.dts that will be > > found in the OF_UPSTREAM directory and modifies it to give it the > > perspective of the micro-controller it will be running on during boot. > > Thanks for bringing this up. I have been playing with the idea to > reuse DT includes from upstream. > > > > > What do you think if we have both paths included regardless if > > OF_UPSTREAM is selected or not? IDK if this will break anyone else > > Sure, we should be able to do that if we maintain the correct order of > include paths as per following patch [1]. If this works for you let me > know and I will include it for v4. > This works beautifully. I did have to hack around to get Makefile.spl working but this is headed in the right direction for me :) Thank you ~Bryan
On Tue, 9 Jan 2024 at 07:24, Bryan Brattlof <bb@ti.com> wrote: > > On January 8, 2024 thus sayeth Sumit Garg: > > Hi Bryan, > > > > On Sat, 6 Jan 2024 at 02:12, Bryan Brattlof <bb@ti.com> wrote: > > > > > > Hi Sumit! > > > > > > On December 28, 2023 thus sayeth Sumit Garg: > > > > Allow u-boot to build DTB from a different directory tree such that > > > > *-u-boot.dtsi files can be included from a common location. Currently > > > > that location is arch/$(ARCH)/dts/, so statically define that common > > > > location. > > > > > > > > This is needed for platform owners to start building DTB files from > > > > devicetree-rebasing directory but still being able to include > > > > *-u-boot.dtsi files. > > > > > > > > Reviewed-by: Tom Rini <trini@konsulko.com> > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > > > --- > > > > > > > > > > ... > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > index 27b9437027c..09330421856 100644 > > > > --- a/scripts/Makefile.lib > > > > +++ b/scripts/Makefile.lib > > > > > > ... > > > > > > > # Uncomment for debugging > > > > @@ -190,6 +192,7 @@ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES) > > > > dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \ > > > > $(UBOOTINCLUDE) \ > > > > -I$(dir $<) \ > > > > + -I$(u_boot_dtsi_loc) \ > > > > -I$(srctree)/arch/$(ARCH)/dts/include \ > > > > -I$(srctree)/include \ > > > > -D__ASSEMBLY__ \ > > > > @@ -328,7 +331,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > > > echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > > > > $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > > > > $(DTC) -O dtb -o $@ -b 0 \ > > > > - -i $(dir $<) $(DTC_FLAGS) \ > > > > + -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > > > > -d $(depfile).dtc.tmp $(dtc-tmp) || \ > > > > (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \ > > > > ; \ > > > > > > One of the issues I see with having a separate OF_UPSTREAM and U-Boot dt > > > directory is when we have U-Boot board files that use dtsi files in the > > > OF_UPSTREAM folder. > > > > > > For example our reference boards uses the primary bootloader's dtb (eg: > > > k3-am62a7-r5-sk.dts) which #includes the k3-am62a7-sk.dts that will be > > > found in the OF_UPSTREAM directory and modifies it to give it the > > > perspective of the micro-controller it will be running on during boot. > > > > Thanks for bringing this up. I have been playing with the idea to > > reuse DT includes from upstream. > > > > > > > > What do you think if we have both paths included regardless if > > > OF_UPSTREAM is selected or not? IDK if this will break anyone else > > > > Sure, we should be able to do that if we maintain the correct order of > > include paths as per following patch [1]. If this works for you let me > > know and I will include it for v4. > > > > This works beautifully. > > I did have to hack around to get Makefile.spl working but this is headed > in the right direction for me :) Thanks for testing. I hope I can take that as a tested-by tag for this patch. -Sumit > > Thank you > ~Bryan
On January 9, 2024 thus sayeth Sumit Garg: > On Tue, 9 Jan 2024 at 07:24, Bryan Brattlof <bb@ti.com> wrote: > > > > On January 8, 2024 thus sayeth Sumit Garg: > > > Hi Bryan, > > > > > > On Sat, 6 Jan 2024 at 02:12, Bryan Brattlof <bb@ti.com> wrote: > > > > > > > > Hi Sumit! > > > > > > > > On December 28, 2023 thus sayeth Sumit Garg: > > > > > Allow u-boot to build DTB from a different directory tree such that > > > > > *-u-boot.dtsi files can be included from a common location. Currently > > > > > that location is arch/$(ARCH)/dts/, so statically define that common > > > > > location. > > > > > > > > > > This is needed for platform owners to start building DTB files from > > > > > devicetree-rebasing directory but still being able to include > > > > > *-u-boot.dtsi files. > > > > > > > > > > Reviewed-by: Tom Rini <trini@konsulko.com> > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > > > > --- > > > > > > > > > > > > > ... > > > > > > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > > index 27b9437027c..09330421856 100644 > > > > > --- a/scripts/Makefile.lib > > > > > +++ b/scripts/Makefile.lib > > > > > > > > ... > > > > > > > > > # Uncomment for debugging > > > > > @@ -190,6 +192,7 @@ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES) > > > > > dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \ > > > > > $(UBOOTINCLUDE) \ > > > > > -I$(dir $<) \ > > > > > + -I$(u_boot_dtsi_loc) \ > > > > > -I$(srctree)/arch/$(ARCH)/dts/include \ > > > > > -I$(srctree)/include \ > > > > > -D__ASSEMBLY__ \ > > > > > @@ -328,7 +331,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > > > > echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > > > > > $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > > > > > $(DTC) -O dtb -o $@ -b 0 \ > > > > > - -i $(dir $<) $(DTC_FLAGS) \ > > > > > + -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > > > > > -d $(depfile).dtc.tmp $(dtc-tmp) || \ > > > > > (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \ > > > > > ; \ > > > > > > > > One of the issues I see with having a separate OF_UPSTREAM and U-Boot dt > > > > directory is when we have U-Boot board files that use dtsi files in the > > > > OF_UPSTREAM folder. > > > > > > > > For example our reference boards uses the primary bootloader's dtb (eg: > > > > k3-am62a7-r5-sk.dts) which #includes the k3-am62a7-sk.dts that will be > > > > found in the OF_UPSTREAM directory and modifies it to give it the > > > > perspective of the micro-controller it will be running on during boot. > > > > > > Thanks for bringing this up. I have been playing with the idea to > > > reuse DT includes from upstream. > > > > > > > > > > > What do you think if we have both paths included regardless if > > > > OF_UPSTREAM is selected or not? IDK if this will break anyone else > > > > > > Sure, we should be able to do that if we maintain the correct order of > > > include paths as per following patch [1]. If this works for you let me > > > know and I will include it for v4. > > > > > > > This works beautifully. > > > > I did have to hack around to get Makefile.spl working but this is headed > > in the right direction for me :) > > Thanks for testing. I hope I can take that as a tested-by tag for this patch. > Absolutely Tested-by: Bryan Brattlof <bb@ti.com> ~Bryan
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 27b9437027c..09330421856 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -159,18 +159,20 @@ cpp_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(UBOOTINCLUDE) \ ld_flags = $(KBUILD_LDFLAGS) $(ldflags-y) $(LDFLAGS_$(@F)) # Try these files in order to find the U-Boot-specific .dtsi include file -u_boot_dtsi_options = $(strip $(wildcard $(dir $<)$(basename $(notdir $<))-u-boot.dtsi) \ - $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \ - $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \ - $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \ - $(wildcard $(dir $<)u-boot.dtsi)) +u_boot_dtsi_loc = $(srctree)/arch/$(ARCH)/dts/ + +u_boot_dtsi_options = $(strip $(wildcard $(u_boot_dtsi_loc)$(basename $(notdir $<))-u-boot.dtsi) \ + $(wildcard $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \ + $(wildcard $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \ + $(wildcard $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \ + $(wildcard $(u_boot_dtsi_loc)u-boot.dtsi)) u_boot_dtsi_options_raw = $(warning Automatic .dtsi inclusion: options: \ - $(dir $<)$(basename $(notdir $<))-u-boot.dtsi \ - $(dir $<)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi \ - $(dir $<)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi \ - $(dir $<)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi \ - $(dir $<)u-boot.dtsi ... \ + $(u_boot_dtsi_loc)$(basename $(notdir $<))-u-boot.dtsi \ + $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi \ + $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi \ + $(u_boot_dtsi_loc)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi \ + $(u_boot_dtsi_loc)u-boot.dtsi ... \ found: $(if $(u_boot_dtsi_options),"$(u_boot_dtsi_options)",nothing!)) # Uncomment for debugging @@ -190,6 +192,7 @@ dtsi_include_list += $(CONFIG_DEVICE_TREE_INCLUDES) dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc \ $(UBOOTINCLUDE) \ -I$(dir $<) \ + -I$(u_boot_dtsi_loc) \ -I$(srctree)/arch/$(ARCH)/dts/include \ -I$(srctree)/include \ -D__ASSEMBLY__ \ @@ -328,7 +331,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ - -i $(dir $<) $(DTC_FLAGS) \ + -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ -d $(depfile).dtc.tmp $(dtc-tmp) || \ (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \ ; \