Patchwork linux: enable ubi and ubifs if we build a ubifs root filesystem

login
register
mail settings
Submitter Fabio Porcedda
Date July 31, 2012, 7:50 a.m.
Message ID <1343721051-8380-1-git-send-email-fabio.porcedda@gmail.com>
Download mbox | patch
Permalink /patch/174172/
State Rejected
Headers show

Comments

Fabio Porcedda - July 31, 2012, 7:50 a.m.
Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 linux/linux.mk | 3 +++
 1 file changed, 3 insertions(+)
Tzu-Jung Lee - July 31, 2012, 9:24 a.m.
On Tue, Jul 31, 2012 at 3:50 PM, Fabio Porcedda
<fabio.porcedda@gmail.com> wrote:
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  linux/linux.mk | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/linux/linux.mk b/linux/linux.mk
> index ca216a3..cf43818 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -166,6 +166,9 @@ define LINUX_CONFIGURE_CMDS
>                 $(call KCONFIG_SET_OPT,CONFIG_UEVENT_HELPER_PATH,\"/sbin/mdev\",$(@D)/.config))
>         $(if $(BR2_PACKAGE_SYSTEMD),
>                 $(call KCONFIG_ENABLE_OPT,CONFIG_CGROUPS,$(@D)/.config))
> +       $(if $(BR2_TARGET_ROOTFS_UBIFS),
> +               $(call KCONFIG_ENABLE_OPT,CONFIG_MTD_UBI,$(@D)/.config)
> +               $(call KCONFIG_ENABLE_OPT,CONFIG_UBIFS_FS,$(@D)/.config))
>         yes '' | $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) oldconfig
>  endef
>

I'm a little bit concerning about post modifications to kernel configs
during the build process (not only this patch).
It may not be so explicit that the kernel was "changed" when the users
were only peaking and poking the root file system.

Since the Kernel may only be tested and released with certain configurations.
It will be helpful if customer feedback on errors could include what
they did explicitly as possible.

Would it be better to issue warnings or even fail-stop the building
process after sanity checks?
If users really need to change the kernel configuration, make sure
they are aware of their changes. :)

Thanks,
Roy
Thomas Petazzoni - July 31, 2012, 7:48 p.m.
Hello,

Thanks for proposing this patch!

Le Tue, 31 Jul 2012 09:50:51 +0200,
Fabio Porcedda <fabio.porcedda@gmail.com> a écrit :

> +	$(if $(BR2_TARGET_ROOTFS_UBIFS),
> +		$(call KCONFIG_ENABLE_OPT,CONFIG_MTD_UBI,$(@D)/.config)
> +		$(call KCONFIG_ENABLE_OPT,CONFIG_UBIFS_FS,$(@D)/.config))

As Tzu-Jung said, the topic of automatically adjusting the kernel
configuration depending on other configuration options is a sensitive
topic. For example: should we enable Netfilter in the kernel if
iptables package is enabled? Should we enable the MTD subsystem in the
kernel if the mtd-utils package is enabled? Since we definitely don't
want to fiddle too much with the kernel configuration of the user, we
trying to limit the number of such automatic tweaks.

At the moment, I'm leaning towards not doing automatic modification of
the kernel configuration depending on the root filesystem type selected
in Buildroot. Let's keep this up to the user. But I'll listen to the
comments of the other developers/users.

Best regards,

Thomas

Patch

diff --git a/linux/linux.mk b/linux/linux.mk
index ca216a3..cf43818 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -166,6 +166,9 @@  define LINUX_CONFIGURE_CMDS
 		$(call KCONFIG_SET_OPT,CONFIG_UEVENT_HELPER_PATH,\"/sbin/mdev\",$(@D)/.config))
 	$(if $(BR2_PACKAGE_SYSTEMD),
 		$(call KCONFIG_ENABLE_OPT,CONFIG_CGROUPS,$(@D)/.config))
+	$(if $(BR2_TARGET_ROOTFS_UBIFS),
+		$(call KCONFIG_ENABLE_OPT,CONFIG_MTD_UBI,$(@D)/.config)
+		$(call KCONFIG_ENABLE_OPT,CONFIG_UBIFS_FS,$(@D)/.config))
 	yes '' | $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) oldconfig
 endef