trivial patch to fix Image name not appearing in 'make menuconfig' (fixex issue #28)
PATCH 1/1 trivial patch to fix Image name not appearing in 'make menuconfig' (fixex issue #28)
I know it's very late for 2015.08, but the patch is really trivial...
I already submitted a bug report upstream, but I have no answer to date, so I suppose local patching is necessary.
PATCH 1/1 trivial patch to fix Image name not appearing in 'make menuconfig' (fixex issue #28)
Oops,
this is (obviously) incomplete.
Update in a few minutes.
Sorry...
Mauro
Il 29/08/2015 12:07, Mauro Condarelli ha scritto:
Quoted 9 lines
I know it's very late for 2015.08, but the patch is really trivial...
I already submitted a bug report upstream, but I have no answer to date, so I suppose local patching is necessary.
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
PATCH 1/1 Fix Image name not configurable (issue #28)
In reply to Mauro CondarelliI hope this time it is ok :)
Mauro
PATCH 1/1 Fix Image name not configurable (issue #28).
--- ...age-name-not-appearing-in-make-menuconfig.patch | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 boot/at91bootstrap3/0001-fix-Image-name-not-appearing-in-make-menuconfig.patch
boot/at91bootstrap3/0001-fix-Image-name-not-appearing-in-make-menuconfig.patch new file +27 -0
@@ -0,0 +1,27 @@ +From d2a65fce095242f28f263bbaba373516b6b4db43 Mon Sep 17 00:00:00 2001 +From: Mauro Condarelli <[email protected]> +Date: Sat, 29 Aug 2015 11:49:26 +0200 +Subject: [PATCH] trivial patch to fix Image name not appearing in 'make + manuconfig' (fixes issue #28). + +Signed-off-by: Mauro Condarelli <[email protected]> +--- + Config.in | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/Config.in b/Config.in +index 432df26..714b05d 100644 +--- a/Config.in ++++ b/Config.in +@@ -81,7 +81,7 @@ source "Config.in.u-boot" + source "Config.in.app-image" + + config CONFIG_IMAGE_NAME +- string ++ string "Image name" + default "Image" if CONFIG_LINUX_IMAGE + default "u-boot.bin" if CONFIG_LOAD_UBOOT + default "softpack.bin" if CONFIG_LOAD_64KB || CONFIG_LOAD_4MB || CONFIG_LOAD_1MB +-- +2.1.4 +
Signature · 1 line
2.1.4
PATCH 1/1 Fix Image name not configurable (issue #28).
Mauro, All,
On 2015-08-29 12:43 +0200, Mauro Condarelli spake thusly:
Quoted 26 lines
Signed-off-by Mauro Condarelli <[email protected]>--- ...age-name-not-appearing-in-make-menuconfig.patch | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 boot/at91bootstrap3/0001-fix-Image-name-not-appearing-in-make-menuconfig.patchboot/at91bootstrap3/0001-fix-Image-name-not-appearing-in-make-menuconfig.patch new file +14
-0@@ -0,0 +1,27 @@ +From d2a65fce095242f28f263bbaba373516b6b4db43 Mon Sep 17 00:00:00 2001 +From: Mauro Condarelli <[email protected]> +Date: Sat, 29 Aug 2015 11:49:26 +0200 +Subject: [PATCH] trivial patch to fix Image name not appearing in 'make + manuconfig' (fixes issue #28). + +Signed-off-by: Mauro Condarelli <[email protected]> +--- + Config.in | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/Config.in b/Config.in +index 432df26..714b05d 100644 +--- a/Config.in
boot/at91bootstrap3/0001-fix-Image-name-not-appearing-in-make-menuconfig.patch line 24
Quoted 10 lines
boot/at91bootstrap3/0001-fix-Image-name-not-appearing-in-make-menuconfig.patch new file +10
-0++++ b/Config.in +@@ -81,7 +81,7 @@ source "Config.in.u-boot" + source "Config.in.app-image" + + config CONFIG_IMAGE_NAME +- string ++ string "Image name" + default "Image" if CONFIG_LINUX_IMAGE + default "u-boot.bin" if CONFIG_LOAD_UBOOT + default "softpack.bin" if CONFIG_LOAD_64KB || CONFIG_LOAD_4MB || CONFIG_LOAD_1MB
As discussed on IRC, this is probably the wrong solution.
A better (and probably more easily upstreamable) solution would be to add a choice for the user to select the type of the image, like so:
In Config.in.kernel:
choice
bool "Image format"
config CONFIG_LINUX_UIMAGE
bool "uImage"
config COMFIG_LINUX_ZIMAGE
bool "zImage"
endchoice
In Config.in:
config CONFIG_IMAGE_NAME
string
default "Image" if CONFIG_LINUX_UIMAGE
default "zImage" if CONFIG_LINUX_ZIMAGE
[...]
Note that CONFIG_LINUX_UIMAGE comes first in the choice, so that a defconfig keeps the existing behaviour of using uImage. You may also want to add support for other image names, YMMV...
I'm marking this patch as rejected in Patchwork.
Regards,
Yann E. MORIN.
Quoted 10 lines
boot/at91bootstrap3/0001-fix-Image-name-not-appearing-in-make-menuconfig.patch new file +3
-0+-- +2.1.4 +Signature · 6 lines
2.1.4 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 Fix Image name not configurable (issue #28).
Yann, All,
After the previous discussion on IRC I came up with this very different patch which does not touch the upstream package at all (even though I still think allowing user to chose isn't evil). Before I try to submit it I would like a comment.
The aim is to force the booted image name according to requested kernel target if (and only if) user contextually requests the image to be installed.
Obvious downside is user has no feedback of the change (and I don't know how to provide this feedback).
Would this be more acceptable?
boot/at91bootstrap3/at91bootstrap3.mk +17 -0
@@ -40,6 +40,23 @@ define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS cp $(@D)/binaries/*.bin $(BINARIES_DIR) endef +ifeq ($(BR2_LINUX_KERNEL_INSTALL_TARGET),y) +define AT91BOOTSTRAP3_KCONFIG_FIXUP_CMDS + $(if $(BR2_LINUX_KERNEL_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_VMLINUX), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"vmlinux",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,$(BR2_LINUX_KERNEL_IMAGE_NAME),$(@D)/.config)) +endef +endif + ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_DEFCONFIG),y) AT91BOOTSTRAP3_SOURCE_CONFIG = $(AT91BOOTSTRAP3_DIR)/board/*/$(call qstrip,$(BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG))_defconfig else ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_CUSTOM_CONFIG),y)
Il 29/08/2015 13:59, Yann E. MORIN ha scritto:
Quoted 2 lines
...
I'm marking this patch as rejected in Patchwork. Regards, Yann E. MORIN.
understood.
Regards
Mauro
PATCH 1/1 Fix Image name not configurable (issue #28).
Dear Mauro Condarelli,
Quoted 21 lines · Mauro Condarelli
Yann, All,
After the previous discussion on IRC I came up with this very different patch which does not touch the upstream package at all (even though I still think allowing user to chose isn't evil). Before I try to submit it I would like a comment.
The aim is to force the booted image name according to requested kernel target if (and only if) user contextually requests the image to be installed.
Obvious downside is user has no feedback of the change (and I don't know how to provide this feedback).
Would this be more acceptable?boot/at91bootstrap3/at91bootstrap3.mk +6
-0@@ -40,6 +40,23 @@ define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS cp $(@D)/binaries/*.bin $(BINARIES_DIR) endef +ifeq ($(BR2_LINUX_KERNEL_INSTALL_TARGET),y) +define AT91BOOTSTRAP3_KCONFIG_FIXUP_CMDS + $(if $(BR2_LINUX_KERNEL_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config))
boot/at91bootstrap3/at91bootstrap3.mk define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS · line 57
Quoted 10 lines
boot/at91bootstrap3/at91bootstrap3.mk +10
-0+ $(if $(BR2_LINUX_KERNEL_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_VMLINUX), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"vmlinux",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,$(BR2_LINUX_KERNEL_IMAGE_NAME),$(@D)/.config)) +endef +endif
I think this is not correct because it assumes you want to boot the Linux kernel directly from ATBootstrap, while many people usually do AT91Bootstrap -> another bootloader (U-Boot or Barebox) -> Linux kernel.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/1 Fix Image name not configurable (issue #28).
Mauro, Thomas, All,
On 2015-08-31 09:32 +0200, Thomas Petazzoni spake thusly:
Quoted 26 lines · Mauro Condarelli
Quoted 25 lines
After the previous discussion on IRC I came up with this very different patch which does not touch the upstream package at all (even though I still think allowing user to chose isn't evil). Before I try to submit it I would like a comment.
The aim is to force the booted image name according to requested kernel target if (and only if) user contextually requests the image to be installed.
Obvious downside is user has no feedback of the change (and I don't know how to provide this feedback).
Would this be more acceptable?boot/at91bootstrap3/at91bootstrap3.mk +12
-0@@ -40,6 +40,23 @@ define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS cp $(@D)/binaries/*.bin $(BINARIES_DIR) endef +ifeq ($(BR2_LINUX_KERNEL_INSTALL_TARGET),y) +define AT91BOOTSTRAP3_KCONFIG_FIXUP_CMDS + $(if $(BR2_LINUX_KERNEL_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_VMLINUX), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"vmlinux",$(@D)/.config))
Quoted 8 lines
boot/at91bootstrap3/at91bootstrap3.mk define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS · line 57
Quoted 4 lines
boot/at91bootstrap3/at91bootstrap3.mk +4
-0+ $(if $(BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,$(BR2_LINUX_KERNEL_IMAGE_NAME),$(@D)/.config)) +endef +endifI think this is not correct because it assumes you want to boot the Linux kernel directly from ATBootstrap, while many people usually do AT91Bootstrap -> another bootloader (U-Boot or Barebox) -> Linux kernel.
Besides, this is changing an option that is not user-configurable, so its lifetime in the .config is not guaranteed. I.e. if the buildsystem of at01bootstrap3 would do an "oldconfig" on it, it would get lost (well, at least that's the case on their master branch).
Please, get the matter with upstream and see what they think.
When upstream has agreed on a "fix", *then* we can see at backporting it until they do a new release.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 Fix Image name not configurable (issue #28).
Agreed.
I will press upstream.
In the meantime I will live with my local changes. I'll keep the list posted on any (eventual) development.
Regards
Mauro
Il 31/08/2015 10:17, Yann E. MORIN ha scritto:
Quoted 49 lines
Mauro, Thomas, All,
On 2015-08-31 09:32 +0200, Thomas Petazzoni spake thusly:
Quoted 25 lines · Mauro Condarelli
Quoted 24 lines
After the previous discussion on IRC I came up with this very different patch which does not touch the upstream package at all (even though I still think allowing user to chose isn't evil). Before I try to submit it I would like a comment.
The aim is to force the booted image name according to requested kernel target if (and only if) user contextually requests the image to be installed.
Obvious downside is user has no feedback of the change (and I don't know how to provide this feedback).
Would this be more acceptable?boot/at91bootstrap3/at91bootstrap3.mk +11
-0@@ -40,6 +40,23 @@ define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS cp $(@D)/binaries/*.bin $(BINARIES_DIR) endef +ifeq ($(BR2_LINUX_KERNEL_INSTALL_TARGET),y) +define AT91BOOTSTRAP3_KCONFIG_FIXUP_CMDS + $(if $(BR2_LINUX_KERNEL_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_VMLINUX),Quoted 8 lines
boot/at91bootstrap3/at91bootstrap3.mk define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS · line 57
Quoted 5 lines
boot/at91bootstrap3/at91bootstrap3.mk +5
-0+ $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"vmlinux",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,$(BR2_LINUX_KERNEL_IMAGE_NAME),$(@D)/.config)) +endef +endifI think this is not correct because it assumes you want to boot the Linux kernel directly from ATBootstrap, while many people usually do AT91Bootstrap -> another bootloader (U-Boot or Barebox) -> Linux kernel.
Besides, this is changing an option that is not user-configurable, so its lifetime in the .config is not guaranteed. I.e. if the buildsystem of at01bootstrap3 would do an "oldconfig" on it, it would get lost (well, at least that's the case on their master branch).
Please, get the matter with upstream and see what they think.
When upstream has agreed on a "fix", *then* we can see at backporting it until they do a new release.
Regards,
Yann E. MORIN.
PATCH 1/1 Fix Image name not configurable (issue #28).
Hi,
I received today (!) answer from upstream:
============= see: https://github.com/linux4sam/at91bootstrap/issues/28#issuecomment-149745536 ==============
Hello,
Yes, it is not configurable, making it configurable is more comfortable. We accepted your advice. Thank you for your support.
Best Regards,
Wenyou Yang
========================================================================================
I thus assume my original (not the more convoluted one I submitted later) patch can be backported.
Please advise about what I should do.
Regards
Mauro Condarelli
Il 31/08/2015 14:48, Mauro Condarelli ha scritto:
Quoted 64 lines
Agreed.
I will press upstream.
In the meantime I will live with my local changes. I'll keep the list posted on any (eventual) development.Regards
MauroIl 31/08/2015 10:17, Yann E. MORIN ha scritto:
Quoted 41 lines
Mauro, Thomas, All,
On 2015-08-31 09:32 +0200, Thomas Petazzoni spake thusly:
Quoted 25 lines · Mauro Condarelli
Quoted 24 lines
After the previous discussion on IRC I came up with this very different patch which does not touch the upstream package at all (even though I still think allowing user to chose isn't evil). Before I try to submit it I would like a comment.
The aim is to force the booted image name according to requested kernel target if (and only if) user contextually requests the image to be installed.
Obvious downside is user has no feedback of the change (and I don't know how to provide this feedback).
Would this be more acceptable?boot/at91bootstrap3/at91bootstrap3.mk +11
-0@@ -40,6 +40,23 @@ define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS cp $(@D)/binaries/*.bin $(BINARIES_DIR) endef +ifeq ($(BR2_LINUX_KERNEL_INSTALL_TARGET),y) +define AT91BOOTSTRAP3_KCONFIG_FIXUP_CMDS + $(if $(BR2_LINUX_KERNEL_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_UIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"uImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_APPENDED_ZIMAGE), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"zImage",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_VMLINUX),Quoted 8 lines
boot/at91bootstrap3/at91bootstrap3.mk define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS · line 57
Quoted 5 lines
boot/at91bootstrap3/at91bootstrap3.mk +5
-0+ $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,"vmlinux",$(@D)/.config)) + $(if $(BR2_LINUX_KERNEL_IMAGE_TARGET_CUSTOM), + $(call KCONFIG_SET_OPT,CONFIG_IMAGE_NAME,$(BR2_LINUX_KERNEL_IMAGE_NAME),$(@D)/.config)) +endef +endifI think this is not correct because it assumes you want to boot the Linux kernel directly from ATBootstrap, while many people usually do AT91Bootstrap -> another bootloader (U-Boot or Barebox) -> Linux kernel.
Besides, this is changing an option that is not user-configurable, so its lifetime in the .config is not guaranteed. I.e. if the buildsystem of at01bootstrap3 would do an "oldconfig" on it, it would get lost (well, at least that's the case on their master branch).
Quoted 8 lines
Please, get the matter with upstream and see what they think.
When upstream has agreed on a "fix", *then* we can see at backporting it until they do a new release.
Regards,
Yann E. MORIN._______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
PATCH 1/1 Fix Image name not configurable (issue #28).
Dear Mauro Condarelli,
Quoted 9 lines · Mauro Condarelli
Hi,
I received today (!) answer from upstream:
============= see: https://github.com/linux4sam/at91bootstrap/issues/28#issuecomment-149745536 ==============Hello,
Yes, it is not configurable, making it configurable is more comfortable. We accepted your advice. Thank you for your support.
Quoted 7 lines
Best Regards,
Wenyou Yang========================================================================================
I thus assume my original (not the more convoluted one I submitted later) patch can be backported.Please advise about what I should do.
You need to get the patch actually accepted upstream, and once it is accepted, backport it into Buildroot (as a temporary solution until upstream makes a new release that includes your change).
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com