| Message ID | 1533313674-722-1-git-send-email-bos@je-eigen-domein.nl |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] linux: build default dtbs if no specific source files given | expand |
Hello Floris, Thanks for this contribution. It definitely makes sense to support this. On Fri, 3 Aug 2018 18:27:54 +0200, Floris Bos wrote: > Currently if one wants to build dtbs, one has to specify which > dts source files to build exactly. > This is inconvenient, and some platforms use non-standard file > names that are currently not supported by buildroot. > (e.g. the overlay files in downstream Raspberry Pi kernel) Could you give some more specific example of those non-standard names ? Are we talking about .dtbo files ? > diff --git a/linux/Config.in b/linux/Config.in > index d30489e..40d337d 100644 > --- a/linux/Config.in > +++ b/linux/Config.in > @@ -377,6 +377,10 @@ config BR2_LINUX_KERNEL_INTREE_DTS_NAME > the trailing .dts. You can provide a list of > dts files to build, separated by spaces. > > + If both this option and out-of-tree path is left empty > + the default set of dtbs for the kernel configuration > + is build. I think this should rather be in the help text of the BR2_LINUX_KERNEL_DTS_SUPPORT. Basically, if you enable BR2_LINUX_KERNEL_DTS_SUPPORT, but leave BR2_LINUX_KERNEL_INTREE_DTS_NAME and BR2_LINUX_KERNEL_CUSTOM_DTS_PATH empty, you'll have all Device Tree for the selected configuration built and installed. As a separate patch, we should also change: bool "Build a Device Tree Blob (DTB)" to bool "Build Device Tree Blobs (DTBs)" > ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y) > ifeq ($(BR2_LINUX_KERNEL_DTB_IS_SELF_BUILT),) > +ifeq ($(strip $(LINUX_DTS_NAME)),) > +define LINUX_BUILD_DTB > + $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) dtbs Perhaps we can do: $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) \ $(if $(LINUX_DTS_NAME),$(LINUX_DTBS),dtbs) to handle both situations at once. > +define LINUX_INSTALL_DTB > + $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) \ > + INSTALL_DTBS_PATH=$(1) dtbs_install I see one problem with see: no all architecture that support building dtbs have the dtbs_install target. Can we install all *.dtb* for example ? Or should we say we don't care if this new feature only works for a subset of the CPU architectures that use DT ? Thanks! Thomas
Hi, On 08/16/2018 10:34 PM, Thomas Petazzoni wrote: > Hello Floris, > > Thanks for this contribution. It definitely makes sense to support this. > > On Fri, 3 Aug 2018 18:27:54 +0200, Floris Bos wrote: >> Currently if one wants to build dtbs, one has to specify which >> dts source files to build exactly. >> This is inconvenient, and some platforms use non-standard file >> names that are currently not supported by buildroot. >> (e.g. the overlay files in downstream Raspberry Pi kernel) > Could you give some more specific example of those non-standard names ? > Are we talking about .dtbo files ? Yes. Name of source file doesn't match installed file. overlays/dwc2-overlay.dts -> overlays/dwc2.dbto > I see one problem with see: no all architecture that support building > dtbs have the dtbs_install target. Would consider that a bug in the architecture's Makefile. > Can we install all *.dtb* for example ? Don't think that will deal with the "-overlay" in my example. Yours sincerely, Floris Bos
On 16/08/2018 23:11, Floris Bos wrote: > Hi, > > On 08/16/2018 10:34 PM, Thomas Petazzoni wrote: >> Hello Floris, >> >> Thanks for this contribution. It definitely makes sense to support this. >> >> On Fri, 3 Aug 2018 18:27:54 +0200, Floris Bos wrote: >>> Currently if one wants to build dtbs, one has to specify which >>> dts source files to build exactly. >>> This is inconvenient, and some platforms use non-standard file >>> names that are currently not supported by buildroot. >>> (e.g. the overlay files in downstream Raspberry Pi kernel) >> Could you give some more specific example of those non-standard names ? >> Are we talking about .dtbo files ? > > Yes. > Name of source file doesn't match installed file. > > overlays/dwc2-overlay.dts -> overlays/dwc2.dbto > >> I see one problem with see: no all architecture that support building >> dtbs have the dtbs_install target. > > Would consider that a bug in the architecture's Makefile. powerpc is the oldest architecture using dtb, and it doesn't have it... The builddeb script has an explicit if grep -q dtbs_install "${srctree}/arch/$SRCARCH/Makefile"; then ... > >> Can we install all *.dtb* for example ? > > Don't think that will deal with the "-overlay" in my example. Yes it will, because the binary to install will be called .dtbo so it matches the *.dtb* pattern. Regards, Arnout > > > > Yours sincerely, > > Floris Bos > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
On 08/22/2018 12:39 AM, Arnout Vandecappelle wrote: > > On 16/08/2018 23:11, Floris Bos wrote: >> Hi, >> >> On 08/16/2018 10:34 PM, Thomas Petazzoni wrote: >>> Hello Floris, >>> >>> Thanks for this contribution. It definitely makes sense to support this. >>> >>> On Fri, 3 Aug 2018 18:27:54 +0200, Floris Bos wrote: >>>> Currently if one wants to build dtbs, one has to specify which >>>> dts source files to build exactly. >>>> This is inconvenient, and some platforms use non-standard file >>>> names that are currently not supported by buildroot. >>>> (e.g. the overlay files in downstream Raspberry Pi kernel) >>> Could you give some more specific example of those non-standard names ? >>> Are we talking about .dtbo files ? >> Yes. >> Name of source file doesn't match installed file. >> >> overlays/dwc2-overlay.dts -> overlays/dwc2.dbto >> >>> I see one problem with see: no all architecture that support building >>> dtbs have the dtbs_install target. >> Would consider that a bug in the architecture's Makefile. > powerpc is the oldest architecture using dtb, and it doesn't have it... > > The builddeb script has an explicit > if grep -q dtbs_install "${srctree}/arch/$SRCARCH/Makefile"; then ... Well, the top level Makefile does clearly define the INSTALL_DTBS_PATH variable: https://github.com/torvalds/linux/blob/master/Makefile#L891 Would argue that if architectures do not provide a target that is able to use that, the architecture's Makefile is broken. Does powerpc provide a dtbs target that builds all dtb, or is that also missing? Anway, as of yesterday there seem to be some progress on consolidating the device-tree building for all architectures, so that should fix things soonish: https://lkml.org/lkml/2018/8/21/763 > >>> Can we install all *.dtb* for example ? >> Don't think that will deal with the "-overlay" in my example. > Yes it will, because the binary to install will be called .dtbo so it matches > the *.dtb* pattern. Keep in mind that the .dtbo will not be in arch/arm/boot/dts but in arch/arm/boot/dts/overlays though. So you still would need something more sophisticated that also takes subdirectories into mind. Yours sincerely, Floris Bos
diff --git a/linux/Config.in b/linux/Config.in index d30489e..40d337d 100644 --- a/linux/Config.in +++ b/linux/Config.in @@ -377,6 +377,10 @@ config BR2_LINUX_KERNEL_INTREE_DTS_NAME the trailing .dts. You can provide a list of dts files to build, separated by spaces. + If both this option and out-of-tree path is left empty + the default set of dtbs for the kernel configuration + is build. + config BR2_LINUX_KERNEL_CUSTOM_DTS_PATH string "Out-of-tree Device Tree Source file paths" help diff --git a/linux/linux.mk b/linux/linux.mk index 7527b11..a00029f 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -341,6 +341,15 @@ endef ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y) ifeq ($(BR2_LINUX_KERNEL_DTB_IS_SELF_BUILT),) +ifeq ($(strip $(LINUX_DTS_NAME)),) +define LINUX_BUILD_DTB + $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) dtbs +endef +define LINUX_INSTALL_DTB + $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) \ + INSTALL_DTBS_PATH=$(1) dtbs_install +endef +else # LINUX_DTS_NAME define LINUX_BUILD_DTB $(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_DTBS) endef @@ -353,6 +362,7 @@ define LINUX_INSTALL_DTB $(1) endef endif # BR2_LINUX_KERNEL_APPENDED_DTB +endif # LINUX_DTS_NAME endif # BR2_LINUX_KERNEL_DTB_IS_SELF_BUILT endif # BR2_LINUX_KERNEL_DTS_SUPPORT @@ -501,8 +511,8 @@ $(error No kernel configuration file specified, check your BR2_LINUX_KERNEL_CUST endif endif -ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT):$(strip $(LINUX_DTS_NAME)),y:) -$(error No kernel device tree source specified, check your \ +ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB):$(strip $(LINUX_DTS_NAME)),y:) +$(error If appending dtb to kernel, device tree source needs to be specified, check your \ BR2_LINUX_KERNEL_INTREE_DTS_NAME / BR2_LINUX_KERNEL_CUSTOM_DTS_PATH settings) endif
Currently if one wants to build dtbs, one has to specify which dts source files to build exactly. This is inconvenient, and some platforms use non-standard file names that are currently not supported by buildroot. (e.g. the overlay files in downstream Raspberry Pi kernel) Fix this by allowing the DTS source to be left empty, in which case the standard list of dtbs for the kernel configuration will be build ("make dtbs") and installed ("make dtbs_install") Only corner case in which specifying file names is still required is when appending dtb to kernel. Signed-off-by: Floris Bos <bos@je-eigen-domein.nl> --- linux/Config.in | 4 ++++ linux/linux.mk | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)