Message ID | 1399904527-8159-1-git-send-email-eric.jarrige@armadeus.org |
---|---|
State | Rejected |
Headers | show |
Dear Eric Jarrige, On Mon, 12 May 2014 16:22:07 +0200, Eric Jarrige wrote: > Add an option BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE that makes it possible > to override the configuration options in the board header file. This > avoids the need for manipulating the board header file with sed hacks > like is currently done for the BR2_TARGET_UBOOT_NETWORK settings. > > Note that this option does not make it possible to add a new board to > U-Boot. That still has to be done by patching the source. > > Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org> > --- > > Changes v2 -> v3: > - refactoring based on uClibc/BusyBox model (suggested by Thomas and Arnoult) > - add missing help text (suggested by Arnoult) > - copy custom configuration file to legal info (suggested by Yann) > Changes v1 -> v2: > - Fix typo > > boot/uboot/Config.in | 8 ++++++++ > boot/uboot/uboot.mk | 18 ++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in > index 1c77f6a..299d50f 100644 > --- a/boot/uboot/Config.in > +++ b/boot/uboot/Config.in > @@ -79,6 +79,14 @@ config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR > > Most users may leave this empty > > +config BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE > + string "Configuration file path" > + help > + Path to the modified configuration header file. It will be > + copied to include/configs/<boardname>.h in the U-Boot sources. > + Note that this doesn't allow you to create a custom board, > + only to modify some of configuration variables. I must say I'm still unconvinced by this patch. In U-Boot, the configuration is completely mixed with the definition of the board, and defining a board does not only require adding a file in include/configs/, but also adding an entry in boards.cfg. Therefore, I really believe that people willing to customize their U-Boot board or configuration should instead carry a patch against U-Boot instead of using this non-standard solution. Moreover, it looks like U-Boot is progressively moving towards a kbuild/kconfig approach (kbuild step is done, I guess kconfig is the next step), which should on the long run make U-Boot closer to the Linux kernel or Barebox in terms of configuration. Best regards, Thomas
Dear Eric Jarrige, On Mon, 12 May 2014 16:22:07 +0200, Eric Jarrige wrote: > Add an option BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE that makes it possible > to override the configuration options in the board header file. This > avoids the need for manipulating the board header file with sed hacks > like is currently done for the BR2_TARGET_UBOOT_NETWORK settings. > > Note that this option does not make it possible to add a new board to > U-Boot. That still has to be done by patching the source. > > Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org> We just had a discussion on IRC with Peter Korsgaard, and he also doesn't like much this solution: 22:16 < Jacmet> kos_tom: I also don't like 348092 He hasn't expressed the details on IRC, but I believe he shares similar concerns to mine, i.e: """ I must say I'm still unconvinced by this patch. In U-Boot, the configuration is completely mixed with the definition of the board, and defining a board does not only require adding a file in include/configs/, but also adding an entry in boards.cfg. Therefore, I really believe that people willing to customize their U-Boot board or configuration should instead carry a patch against U-Boot instead of using this non-standard solution. """ Of course, if Peter has different/other reasons, I'm sure he will give more details in this thread. As a consequence, I'm marking the patch as Rejected in patchwork. Thanks, Thomas
Hi Thomas, On 11 juin 2014, at 22:16, Thomas Petazzoni wrote: > > Dear Eric Jarrige, > > On Mon, 12 May 2014 16:22:07 +0200, Eric Jarrige wrote: >> Add an option BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE that makes it possible >> to override the configuration options in the board header file. This >> avoids the need for manipulating the board header file with sed hacks >> like is currently done for the BR2_TARGET_UBOOT_NETWORK settings. >> >> Note that this option does not make it possible to add a new board to >> U-Boot. That still has to be done by patching the source. >> >> Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org> > > We just had a discussion on IRC with Peter Korsgaard, and he also > doesn't like much this solution: > > 22:16 < Jacmet> kos_tom: I also don't like 348092 > > He hasn't expressed the details on IRC, but I believe he shares similar > concerns to mine, i.e: > > """ > I must say I'm still unconvinced by this patch. In U-Boot, the > configuration is completely mixed with the definition of the board, and > defining a board does not only require adding a file in > include/configs/, but also adding an entry in boards.cfg. > > Therefore, I really believe that people willing to customize their > U-Boot board or configuration should instead carry a patch against > U-Boot instead of using this non-standard solution. > """ > > Of course, if Peter has different/other reasons, I'm sure he will > give more details in this thread. > > As a consequence, I'm marking the patch as Rejected in patchwork. > > Thanks, Thanks for your feedback. Regards, Eric
diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in index 1c77f6a..299d50f 100644 --- a/boot/uboot/Config.in +++ b/boot/uboot/Config.in @@ -79,6 +79,14 @@ config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR Most users may leave this empty +config BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE + string "Configuration file path" + help + Path to the modified configuration header file. It will be + copied to include/configs/<boardname>.h in the U-Boot sources. + Note that this doesn't allow you to create a custom board, + only to modify some of configuration variables. + choice prompt "U-Boot binary format" default BR2_TARGET_UBOOT_FORMAT_BIN diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk index da67706..1fa3fb9 100644 --- a/boot/uboot/uboot.mk +++ b/boot/uboot/uboot.mk @@ -94,7 +94,25 @@ endef UBOOT_POST_PATCH_HOOKS += UBOOT_APPLY_CUSTOM_PATCHES endif +UBOOT_CUSTOM_CONFIG = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE)) + +ifneq ($(UBOOT_CUSTOM_CONFIG),) + +define UBOOT_COPY_CUSTOM_CONFIG_FILE + $(INSTALL) -m 0644 $(UBOOT_CUSTOM_CONFIG) \ + $(@D)/include/configs/$(UBOOT_BOARD_NAME).h; +endef + +define UBOOT_COPY_CUSTOM_CONFIG_FILE_TO_LEGAL_INFO + $(INSTALL) -m 0644 $(UBOOT_CUSTOM_CONFIG) \ + $(REDIST_SOURCES_DIR_TARGET)/$(UBOOT_BOARD_NAME).h; +endef + +UBOOT_POST_LEGAL_INFO_HOOKS += UBOOT_COPY_CUSTOM_CONFIG_FILE_TO_LEGAL_INFO +endif + define UBOOT_CONFIGURE_CMDS + $(UBOOT_COPY_CUSTOM_CONFIG_FILE) $(TARGET_CONFIGURE_OPTS) $(UBOOT_CONFIGURE_OPTS) \ $(MAKE) -C $(@D) $(UBOOT_MAKE_OPTS) \ $(UBOOT_BOARD_NAME)_config
Add an option BR2_TARGET_UBOOT_CUSTOM_CONFIG_FILE that makes it possible to override the configuration options in the board header file. This avoids the need for manipulating the board header file with sed hacks like is currently done for the BR2_TARGET_UBOOT_NETWORK settings. Note that this option does not make it possible to add a new board to U-Boot. That still has to be done by patching the source. Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org> --- Changes v2 -> v3: - refactoring based on uClibc/BusyBox model (suggested by Thomas and Arnoult) - add missing help text (suggested by Arnoult) - copy custom configuration file to legal info (suggested by Yann) Changes v1 -> v2: - Fix typo boot/uboot/Config.in | 8 ++++++++ boot/uboot/uboot.mk | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+)