| Message ID | e330f5c885f762feb6f4d7487fe3f6aa29c7fca3.1420142707.git.yann.morin.1998@free.fr |
|---|---|
| State | Rejected |
| Headers | show |
Dear Yann E. MORIN, On Thu, 1 Jan 2015 21:23:48 +0100, Yann E. MORIN wrote: > On some platforms (e.g. the Raspberry Pi), the DTB is already bundled > with the bootloader, and installed with it. On some other platforms, the > DTB might even be stored on the device itself (which is the ultimate > goal of the DTB). > > So far, when we build a kernel with DT support, we forcibly want a to > also build the DTB blob, which goes against the situations described > above. > > Add a new option in the DT choice, to specify the DTB will be provided > by the bootloader or the device itself. > > This also means we must be a little bit more selective when we check the > validity of the DTS name at build time. > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> I don't really see the point of this. If you don't want to build a DTB, then don't tell Buildroot that you want to build a DTB, and that's it. Why would we want such a patch? Enabling CONFIG_OF should be done by the platform defconfig. It really is tied to how the platform is supported in the Linux kernel, and is not something that Buildroot should touch IMO. Thanks, Thomas
Thomas, All, On 2015-01-01 22:24 +0100, Thomas Petazzoni spake thusly: > On Thu, 1 Jan 2015 21:23:48 +0100, Yann E. MORIN wrote: > > On some platforms (e.g. the Raspberry Pi), the DTB is already bundled > > with the bootloader, and installed with it. On some other platforms, the > > DTB might even be stored on the device itself (which is the ultimate > > goal of the DTB). > > > > So far, when we build a kernel with DT support, we forcibly want a to > > also build the DTB blob, which goes against the situations described > > above. > > > > Add a new option in the DT choice, to specify the DTB will be provided > > by the bootloader or the device itself. > > > > This also means we must be a little bit more selective when we check the > > validity of the DTS name at build time. > > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > I don't really see the point of this. If you don't want to build a DTB, > then don't tell Buildroot that you want to build a DTB, and that's it. > Why would we want such a patch? > > Enabling CONFIG_OF should be done by the platform defconfig. It really > is tied to how the platform is supported in the Linux kernel, and is > not something that Buildroot should touch IMO. Fact is, the prompt reads: [*] Device tree support So, I take that as "build a kernel with support for the device tree", not as "build a DTB". But Now I think of it, this patch is doing two things: - enable USE_OF when that option is enabled, - add support for bootloader- or board-provided DTB. Maybe that should be split up in two, then? Or do you still want that option to mean "build a DTB" ? Regards, Yann E. MORIN.
Dear Yann E. MORIN, On Thu, 1 Jan 2015 22:44:35 +0100, Yann E. MORIN wrote: > Fact is, the prompt reads: > > [*] Device tree support > > So, I take that as "build a kernel with support for the device tree", > not as "build a DTB". Right, the prompt may not be the most appropriate one. > But Now I think of it, this patch is doing two things: > > - enable USE_OF when that option is enabled, > > - add support for bootloader- or board-provided DTB. > > Maybe that should be split up in two, then? > > Or do you still want that option to mean "build a DTB" ? Yes, I believe the prompt should be changed. I really don't see the point of adding a new entry in the choice to simply mean "do nothing". And also, enabling CONFIG_OF should be done by the defconfig of the platform, not by Buildroot. Thanks, Thomas
Thomas, All, On 2015-01-01 22:51 +0100, Thomas Petazzoni spake thusly: > On Thu, 1 Jan 2015 22:44:35 +0100, Yann E. MORIN wrote: > > > Fact is, the prompt reads: > > > > [*] Device tree support > > > > So, I take that as "build a kernel with support for the device tree", > > not as "build a DTB". > > Right, the prompt may not be the most appropriate one. OK. > > But Now I think of it, this patch is doing two things: > > > > - enable USE_OF when that option is enabled, > > > > - add support for bootloader- or board-provided DTB. > > > > Maybe that should be split up in two, then? > > > > Or do you still want that option to mean "build a DTB" ? > > Yes, I believe the prompt should be changed. I really don't see the > point of adding a new entry in the choice to simply mean "do nothing". > > And also, enabling CONFIG_OF should be done by the defconfig of the > platform, not by Buildroot. OK, thanks for the position statement. However, I beg to disagree on that last part: I think we should have this as an option in Buildroot. The reason is that some platforms can be built with or without support for the device tree. The Raspberry Pi is but one such platform; there are others. Their defconfig do not always enable USE_OF by default, it must be a volunteer selection from the user. In that case, your opinion is to delegate to the user the responsibility to enable that on his own, right? So, it would no longer be possible to use in-tree defconfigs for those boards, for example. But as a starter, I'll rename the prompt, so it is no longer ambiguous. Regards, Yann E. MORIN.
Dear Yann E. MORIN, On Thu, 1 Jan 2015 22:58:45 +0100, Yann E. MORIN wrote: > However, I beg to disagree on that last part: I think we should have > this as an option in Buildroot. > > The reason is that some platforms can be built with or without support > for the device tree. The Raspberry Pi is but one such platform; there > are others. Their defconfig do not always enable USE_OF by default, it > must be a volunteer selection from the user. > > In that case, your opinion is to delegate to the user the responsibility > to enable that on his own, right? So, it would no longer be possible to > use in-tree defconfigs for those boards, for example. Well, to me this is a slippery slope. There are gazillions of kernel options that do platform-specific stuff. So far, we really tried hard to limit how much Buildroot modifies the kernel options, and tried to limit that to cases related to userspace components enabled by the user that really need some feature in the kernel. Let's see what Peter thinks about this, he might very well have a different opinion on this. Best regards, Thomas
Thomas, All, On 2015-01-01 23:17 +0100, Thomas Petazzoni spake thusly: > Dear Yann E. MORIN, > > On Thu, 1 Jan 2015 22:58:45 +0100, Yann E. MORIN wrote: > > > However, I beg to disagree on that last part: I think we should have > > this as an option in Buildroot. > > > > The reason is that some platforms can be built with or without support > > for the device tree. The Raspberry Pi is but one such platform; there > > are others. Their defconfig do not always enable USE_OF by default, it > > must be a volunteer selection from the user. > > > > In that case, your opinion is to delegate to the user the responsibility > > to enable that on his own, right? So, it would no longer be possible to > > use in-tree defconfigs for those boards, for example. > > Well, to me this is a slippery slope. There are gazillions of kernel > options that do platform-specific stuff. So far, we really tried hard > to limit how much Buildroot modifies the kernel options, and tried to > limit that to cases related to userspace components enabled by the user > that really need some feature in the kernel. Ok, it all makes sense. I agree, we should be pretty conservative about what we're doing there. > Let's see what Peter thinks about this, he might very well have a > different opinion on this. Yup. In the meantime, I'll rewrite the rest of the series to take this into consideration. It's a few pretty trivial changes. Adding what I wanted can very well be done later if Peter sees it fit to have. The big remaining issue I can anyway work around outside Buildroot, since in my case I do provide the Linux defconfig file, so I'm prefectly OK with ditching this for now. Thank you! :-) Regards, Yann E. MORIN.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: Hi, > Dear Yann E. MORIN, > On Thu, 1 Jan 2015 22:58:45 +0100, Yann E. MORIN wrote: >> However, I beg to disagree on that last part: I think we should have >> this as an option in Buildroot. >> >> The reason is that some platforms can be built with or without support >> for the device tree. The Raspberry Pi is but one such platform; there >> are others. Their defconfig do not always enable USE_OF by default, it >> must be a volunteer selection from the user. >> >> In that case, your opinion is to delegate to the user the responsibility >> to enable that on his own, right? So, it would no longer be possible to >> use in-tree defconfigs for those boards, for example. > Well, to me this is a slippery slope. There are gazillions of kernel > options that do platform-specific stuff. So far, we really tried hard > to limit how much Buildroot modifies the kernel options, and tried to > limit that to cases related to userspace components enabled by the user > that really need some feature in the kernel. > Let's see what Peter thinks about this, he might very well have a > different opinion on this. I must say I'm with Thomas on this one. Let's not fiddle with the kernel .config unless we really _HAVE_ to.
Peter, Thomas, All, On 2015-01-02 00:15 +0100, Peter Korsgaard spake thusly: > >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > > Dear Yann E. MORIN, > > On Thu, 1 Jan 2015 22:58:45 +0100, Yann E. MORIN wrote: > > >> However, I beg to disagree on that last part: I think we should have > >> this as an option in Buildroot. > >> > >> The reason is that some platforms can be built with or without support > >> for the device tree. The Raspberry Pi is but one such platform; there > >> are others. Their defconfig do not always enable USE_OF by default, it > >> must be a volunteer selection from the user. > >> > >> In that case, your opinion is to delegate to the user the responsibility > >> to enable that on his own, right? So, it would no longer be possible to > >> use in-tree defconfigs for those boards, for example. > > > Well, to me this is a slippery slope. There are gazillions of kernel > > options that do platform-specific stuff. So far, we really tried hard > > to limit how much Buildroot modifies the kernel options, and tried to > > limit that to cases related to userspace components enabled by the user > > that really need some feature in the kernel. > > > Let's see what Peter thinks about this, he might very well have a > > different opinion on this. > > I must say I'm with Thomas on this one. Let's not fiddle with the kernel > .config unless we really _HAVE_ to. OK, marked as Rejected in Patchwork, dropped from my tree. Thamks! :-) Regards, Yann E. MORIN.
diff --git a/linux/Config.in b/linux/Config.in index ba4b574..6d994ee 100644 --- a/linux/Config.in +++ b/linux/Config.in @@ -313,6 +313,14 @@ config BR2_LINUX_KERNEL_USE_CUSTOM_DTS Use a custom device tree file, i.e, a device tree file that does not belong to the kernel source tree. + +config BR2_LINUX_KERNEL_USE_EXTERNAL_DTS + bool "Use a bootloader-provided device tree blob" + help + Say 'y' here if your device tree blob is already + bundled by the bootloader, or already present on + the board. + endchoice config BR2_LINUX_KERNEL_INTREE_DTS_NAME diff --git a/linux/linux.mk b/linux/linux.mk index 8256641..9b1f19b 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -59,16 +59,20 @@ LINUX_MAKE_FLAGS = \ # going to be installed in the target filesystem. LINUX_VERSION_PROBED = $(shell $(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) --no-print-directory -s kernelrelease) +ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y) + ifeq ($(BR2_LINUX_KERNEL_USE_INTREE_DTS),y) KERNEL_DTS_NAME = $(call qstrip,$(BR2_LINUX_KERNEL_INTREE_DTS_NAME)) else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y) KERNEL_DTS_NAME = $(basename $(notdir $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)))) endif -ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y) +ifeq ($(BR2_LINUX_KERNEL_USE_INTREE_DTS)$(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y) +ifeq ($(KERNEL_DTS_NAME),) $(error No kernel device tree source specified, check your \ BR2_LINUX_KERNEL_USE_INTREE_DTS / BR2_LINUX_KERNEL_USE_CUSTOM_DTS settings) endif +endif ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y) ifneq ($(words $(KERNEL_DTS_NAME)),1) @@ -79,6 +83,8 @@ endif KERNEL_DTBS = $(addsuffix .dtb,$(KERNEL_DTS_NAME)) +endif # BR2_LINUX_KERNEL_DTS_SUPPORT == y + ifeq ($(BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM),y) LINUX_IMAGE_NAME = $(call qstrip,$(BR2_LINUX_KERNEL_IMAGE_NAME)) LINUX_TARGET_NAME = $(call qstrip,$(BR2_LINUX_KERNEL_IMAGE_TARGET_NAME)) @@ -212,12 +218,15 @@ define LINUX_CONFIGURE_CMDS $(call KCONFIG_ENABLE_OPT,CONFIG_SECURITY,$(@D)/.config) $(call KCONFIG_ENABLE_OPT,CONFIG_SECURITY_SMACK,$(@D)/.config) $(call KCONFIG_ENABLE_OPT,CONFIG_SECURITY_NETWORK,$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_DTS_SUPPORT), + $(call KCONFIG_ENABLE_OPT,CONFIG_USE_OF,$(@D)/.config)) $(if $(BR2_LINUX_KERNEL_APPENDED_DTB), $(call KCONFIG_ENABLE_OPT,CONFIG_ARM_APPENDED_DTB,$(@D)/.config)) yes '' | $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) oldconfig endef ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y) +ifeq ($(BR2_LINUX_KERNEL_USE_EXTERNAL_DTS),) ifeq ($(BR2_LINUX_KERNEL_DTB_IS_SELF_BUILT),) define LINUX_BUILD_DTB $(TARGET_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(KERNEL_DTBS) @@ -238,6 +247,7 @@ define LINUX_INSTALL_DTB_TARGET endef endif endif +endif ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y) # dtbs moved from arch/$ARCH/boot to arch/$ARCH/boot/dts since 3.8-rc1
On some platforms (e.g. the Raspberry Pi), the DTB is already bundled with the bootloader, and installed with it. On some other platforms, the DTB might even be stored on the device itself (which is the ultimate goal of the DTB). So far, when we build a kernel with DT support, we forcibly want a to also build the DTB blob, which goes against the situations described above. Add a new option in the DT choice, to specify the DTB will be provided by the bootloader or the device itself. This also means we must be a little bit more selective when we check the validity of the DTS name at build time. Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> --- linux/Config.in | 8 ++++++++ linux/linux.mk | 12 +++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)