* [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB @ 2024-01-11 13:19 Laurent Badel via buildroot 2024-01-11 15:26 ` Yann E. MORIN 0 siblings, 1 reply; 6+ messages in thread From: Laurent Badel via buildroot @ 2024-01-11 13:19 UTC (permalink / raw) To: buildroot; +Cc: Laurent Badel When building a zImage with appended DTB, buildroot creates a copy of the zImage named zImage.$(LINUX_DTS_NAME). mxs-bootlets.mk does not take this into consideration and instead passes the original zImage (without DTB appended) to elftosb to generate the SB file. Thus, make sure that the correct zImage is used in this process. Note: this patch only supports a single DTS specified in the configuration, because there is no obvious use case for multiple DTS's with mxs-bootlets. If multiple DTS's are configured, only the first one will be used. Signed-off-by: Laurent Badel <laurentbadel@eaton.com> --- boot/mxs-bootlets/mxs-bootlets.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/boot/mxs-bootlets/mxs-bootlets.mk b/boot/mxs-bootlets/mxs-bootlets.mk index adc22767..2003b23f 100644 --- a/boot/mxs-bootlets/mxs-bootlets.mk +++ b/boot/mxs-bootlets/mxs-bootlets.mk @@ -65,9 +65,10 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ $(MAKE1) -C $(@D) linux_prep endef +ZIMAGE_NAME=zImage$(if $(BR2_LINUX_KERNEL_APPENDED_DTB),.$(notdir $(firstword $(LINUX_DTS_NAME)))) define MXS_BOOTLETS_SED_LINUX sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) + sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/$(ZIMAGE_NAME)";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) endef endif -- 2.17.1 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB 2024-01-11 13:19 [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB Laurent Badel via buildroot @ 2024-01-11 15:26 ` Yann E. MORIN 2024-01-12 8:30 ` Badel, Laurent via buildroot 0 siblings, 1 reply; 6+ messages in thread From: Yann E. MORIN @ 2024-01-11 15:26 UTC (permalink / raw) To: Laurent Badel; +Cc: buildroot Laurent, All, On 2024-01-11 13:19 +0000, Laurent Badel via buildroot spake thusly: > When building a zImage with appended DTB, buildroot creates a copy of > the zImage named zImage.$(LINUX_DTS_NAME). mxs-bootlets.mk does not > take this into consideration and instead passes the original zImage > (without DTB appended) to elftosb to generate the SB file. Thus, > make sure that the correct zImage is used in this process. > > Note: this patch only supports a single DTS specified in the > configuration, because there is no obvious use case for multiple DTS's > with mxs-bootlets. If multiple DTS's are configured, only the first one > will be used. That's a bit unfortunate that we have to rely on the ordering. So, if using two or more DTS in cunjunction with mxs-bootlets does not make sense, should that be forbidden? I.e. would we need something like the following (code slightly elided for brevity): ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) ifeq ($(BR2_BOOT_MXS_BOOTLETS),y) ifneq ($(words $(LINUX_DTS_NAME)),1) $(error More than one DTS specified; mxs-bootlets can only use one) endif endif define MXS_BOOTLETS_BUILD_LINUX_PREP ... endef MXS_BOOTLETS_ZIMAGE_NAME = zImage$(....) define MXS_BOOTLETS_SED_LINUX ... endif endif (See below the name of the variable.) > Signed-off-by: Laurent Badel <laurentbadel@eaton.com> > --- > boot/mxs-bootlets/mxs-bootlets.mk | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/boot/mxs-bootlets/mxs-bootlets.mk b/boot/mxs-bootlets/mxs-bootlets.mk > index adc22767..2003b23f 100644 > --- a/boot/mxs-bootlets/mxs-bootlets.mk > +++ b/boot/mxs-bootlets/mxs-bootlets.mk > @@ -65,9 +65,10 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > $(MAKE1) -C $(@D) linux_prep > endef > +ZIMAGE_NAME=zImage$(if $(BR2_LINUX_KERNEL_APPENDED_DTB),.$(notdir $(firstword $(LINUX_DTS_NAME)))) The namespace is global, so the variable names have to be manually scopped with the name of the package they apply to, in this case: MXS_BOOTLETS_ZIMAGE_NAME Regards, Yann E. MORIN. > define MXS_BOOTLETS_SED_LINUX > sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > + sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/$(ZIMAGE_NAME)";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > endef > endif > > -- > 2.17.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB 2024-01-11 15:26 ` Yann E. MORIN @ 2024-01-12 8:30 ` Badel, Laurent via buildroot 2024-01-12 16:21 ` Yann E. MORIN 0 siblings, 1 reply; 6+ messages in thread From: Badel, Laurent via buildroot @ 2024-01-12 8:30 UTC (permalink / raw) To: Yann E. MORIN; +Cc: buildroot@buildroot.org Yann, All, Thank you very much for your time reviewing/commenting on the patch. I spent some time trying to find a way to support multiple DTSs for the sake of completeness, please see the patch below. I would have preferred to use a single loop for all cases but couldn't find a good way to iterate with an empty string, so this results in slight duplication of code unfortunately. I have tested building mxs-bootlets with the following setups: 1. zImage with appended DTB, two DTSs configured (one in-tree, the other out-of-tree): -> generates 2 SB files with extension .$(dtb_name).sb 2. zImage, no DTS: -> generates a single SB file with the .sb extension. 3. zImage, two DTSs as in (1) but not appended: -> generates a single SB file with the .sb extension I checked the output shell commands and they looked as expected, too. I also minded your comment regarding the naming of variables, thank you for pointing this out. If you think that is a better solution, I am happy to re-submit a clean patch. Best regards, Laurent --- boot/mxs-bootlets/mxs-bootlets.mk | 40 +++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/boot/mxs-bootlets/mxs-bootlets.mk b/boot/mxs-bootlets/mxs-bootlets.mk index adc22767..0a1f7009 100644 --- a/boot/mxs-bootlets/mxs-bootlets.mk +++ b/boot/mxs-bootlets/mxs-bootlets.mk @@ -41,17 +41,17 @@ MXS_BOOTLETS_LICENSE = GPL-2.0+ ifeq ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) MXS_BOOTLETS_DEPENDENCIES += barebox MXS_BOOTLETS_BOOTDESC = barebox$(MXS_BOOTLETS_IVT_SUFFIX).bd -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX).sb +MXS_BOOTLETS_BOOTSTREAM_BASE = $(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX) else ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) MXS_BOOTLETS_DEPENDENCIES += linux MXS_BOOTLETS_BOOTDESC = linux$(MXS_BOOTLETS_IVT_SUFFIX).bd -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX).sb +MXS_BOOTLETS_BOOTSTREAM_BASE = $(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX) else ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) MXS_BOOTLETS_DEPENDENCIES += uboot MXS_BOOTLETS_BOOTDESC = uboot$(MXS_BOOTLETS_IVT_SUFFIX).bd -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX).sb +MXS_BOOTLETS_BOOTSTREAM_BASE = $(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX) endif ifeq ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) @@ -65,11 +65,16 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ $(MAKE1) -C $(@D) linux_prep endef + define MXS_BOOTLETS_SED_LINUX sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) + sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage$(1)";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) endef + +ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB), y) +MXS_BOOTLETS_DTS_NAMES = $(foreach dts_name,$(LINUX_DTS_NAME),.$(strip $(dts_name))) endif +endif #BR2_TARGET_MXS_BOOTLETS_LINUX ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) define MXS_BOOTLETS_SED_UBOOT @@ -83,7 +88,13 @@ endef MXS_BOOTLETS_POST_EXTRACT_HOOKS += MXS_BOOTLETS_INSTALL_BAREBOX_BOOTDESC -define MXS_BOOTLETS_BUILD_CMDS +define MXS_BOOTLETS_ELFTOSB_CMD + $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ + -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ + -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE)$(1).sb +endef + +define MXS_BOOTLETS_BUILD_CMDS BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ $(MAKE1) -C $(@D) power_prep BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ @@ -92,15 +103,24 @@ define MXS_BOOTLETS_BUILD_CMDS sed -i 's,[^ *]power_prep.*;,\tpower_prep="$(@D)/power_prep/power_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) sed -i 's,[^ *]sdram_prep.*;,\tsdram_prep="$(@D)/boot_prep/boot_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) $(MXS_BOOTLETS_SED_BAREBOX) - $(MXS_BOOTLETS_SED_LINUX) $(MXS_BOOTLETS_SED_UBOOT) - $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ - -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ - -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) + $(if $(MXS_BOOTLETS_DTS_NAMES), + $(foreach dts_name, $(MXS_BOOTLETS_DTS_NAMES), + $(call MXS_BOOTLETS_SED_LINUX,$(dts_name)) + $(call MXS_BOOTLETS_ELFTOSB_CMD,$(dts_name)) + ), + $(call MXS_BOOTLETS_SED_LINUX) + $(call MXS_BOOTLETS_ELFTOSB_CMD) + ) endef define MXS_BOOTLETS_INSTALL_TARGET_CMDS - cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) $(BINARIES_DIR)/ + $(if $(MXS_BOOTLETS_DTS_NAMES), + $(foreach dts_name, $(MXS_BOOTLETS_DTS_NAMES), + cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE)$(dts_name).sb $(BINARIES_DIR)/ + ), + cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE).sb $(BINARIES_DIR)/ + ) endef $(eval $(generic-package)) -- 2.17.1 -----Original Message----- From: Yann E. MORIN <yann.morin.1998@free.fr> Sent: Thursday, January 11, 2024 4:26 PM To: Badel, Laurent <LaurentBadel@eaton.com> Cc: buildroot@buildroot.org Subject: [EXTERNAL] Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB Laurent, All, On 2024-01-11 13:19 +0000, Laurent Badel via buildroot spake thusly: > When building a zImage with appended DTB, buildroot creates a copy of > the zImage named zImage.$(LINUX_DTS_NAME). mxs-bootlets.mk does not > take this into consideration and instead passes the original zImage > (without DTB appended) to elftosb to generate the SB file. Thus, make > sure that the correct zImage is used in this process. > > Note: this patch only supports a single DTS specified in the > configuration, because there is no obvious use case for multiple DTS's > with mxs-bootlets. If multiple DTS's are configured, only the first > one will be used. That's a bit unfortunate that we have to rely on the ordering. So, if using two or more DTS in cunjunction with mxs-bootlets does not make sense, should that be forbidden? I.e. would we need something like the following (code slightly elided for brevity): ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) ifeq ($(BR2_BOOT_MXS_BOOTLETS),y) ifneq ($(words $(LINUX_DTS_NAME)),1) $(error More than one DTS specified; mxs-bootlets can only use one) endif endif define MXS_BOOTLETS_BUILD_LINUX_PREP ... endef MXS_BOOTLETS_ZIMAGE_NAME = zImage$(....) define MXS_BOOTLETS_SED_LINUX ... endif endif (See below the name of the variable.) > Signed-off-by: Laurent Badel <laurentbadel@eaton.com> > --- > boot/mxs-bootlets/mxs-bootlets.mk | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/boot/mxs-bootlets/mxs-bootlets.mk > b/boot/mxs-bootlets/mxs-bootlets.mk > index adc22767..2003b23f 100644 > --- a/boot/mxs-bootlets/mxs-bootlets.mk > +++ b/boot/mxs-bootlets/mxs-bootlets.mk > @@ -65,9 +65,10 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > $(MAKE1) -C $(@D) linux_prep endef > +ZIMAGE_NAME=zImage$(if $(BR2_LINUX_KERNEL_APPENDED_DTB),.$(notdir > +$(firstword $(LINUX_DTS_NAME)))) The namespace is global, so the variable names have to be manually scopped with the name of the package they apply to, in this case: MXS_BOOTLETS_ZIMAGE_NAME Regards, Yann E. MORIN. > define MXS_BOOTLETS_SED_LINUX > sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > + sed -i 's,[^ > + *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/$(ZIMAGE_NAME)";,' > + $(@D)/$(MXS_BOOTLETS_BOOTDESC) > endef > endif > > -- > 2.17.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://list/ > s.buildroot.org%2Fmailman%2Flistinfo%2Fbuildroot&data=05%7C02%7Clauren > tbadel%40eaton.com%7C28583d4b11284523498408dc12b9a4b4%7Cd6525c95b90643 > 1ab926e9b51ba43cc4%7C1%7C0%7C638405835765391645%7CUnknown%7CTWFpbGZsb3 > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7 > C3000%7C%7C%7C&sdata=koINDlCH%2Bo4Tv20L6jeEvTllYPrpESfnGcFRL%2F%2BbKu0 > %3D&reserved=0 -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' | conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB 2024-01-12 8:30 ` Badel, Laurent via buildroot @ 2024-01-12 16:21 ` Yann E. MORIN 2024-01-16 9:32 ` Badel, Laurent via buildroot 0 siblings, 1 reply; 6+ messages in thread From: Yann E. MORIN @ 2024-01-12 16:21 UTC (permalink / raw) To: Badel, Laurent; +Cc: buildroot@buildroot.org Laurent, All, On 2024-01-12 08:30 +0000, Badel, Laurent spake thusly: > Thank you very much for your time reviewing/commenting on the patch. > I spent some time trying to find a way to support multiple DTSs for > the sake of completeness, please see the patch below. After a cursory look, it looks quite OK. > I would have preferred to use a single loop for all cases but couldn't > find a good way to iterate with an empty string, so this results in > slight duplication of code unfortunately. Err... I don't have a much better idea either... We could cheat with with some quoting tricks, like adding an item in the list that is just a pair of sngle quotes, which for the _SED macro would close and reopen the sed expression, and for the _ELF_TOSB would be interpreted just as an empty string. That's a dirt trick, though, and quite a bit fragile. I.e. somethinglike (nopte also the use of patsubst instead of foreach): # '' is used to create an empty item: # - closes and reopens the single-quoted string of the sed expression # - opens and closes an empty string in the elftosb output filename MXS_BOOTLETS_DTS_NAMES = '' $(patsubst %,.%,$(LINUX_DTS_NAME)) I'm not sold on the above, because, yes, it's fragile... I've quickly tried a few tricks involving the use of $(space) and/or $(empty), but none did work... > I have tested building mxs-bootlets with the following setups: > 1. zImage with appended DTB, two DTSs configured (one in-tree, the > other out-of-tree): > -> generates 2 SB files with extension .$(dtb_name).sb > 2. zImage, no DTS: > -> generates a single SB file with the .sb extension. > 3. zImage, two DTSs as in (1) but not appended: > -> generates a single SB file with the .sb extension Cool, thanks for the effort in looking into, and in testing, that! > I checked the output shell commands and they looked as expected, too. > I also minded your comment regarding the naming of variables, thank you > for pointing this out. > > If you think that is a better solution, I am happy to re-submit a clean > patch. Well, just looking at the code, it looks better, because we do not rely on the ordering of the DTS names to generate one mxs-bootlet, aa we get as many mxs-bootlets as there are configured DTS. However, I have no idea if it makes sense to have as many bootlets as there are DTS names; you are better placed to know that. Sorry if my review seemed misled you into this change. What my point in my review was, was two fold: - _if_ it does not make sense to have multiple DTS configured when using mxs-bootlets, then the situation must be caught and the user must be warned about that (hence my code snippet to detect the case); - otherwise, _if_ it is OK to have multiple DTS configured, and that it is OK to build a single mxs-bootlet in that case, then it is sad to rely on the ordering (but I did not provide any suggestion to fix that, sorry; maybe just a comment stating so in the help text for BR2_TARGET_MXS_BOOTLETS_LINUX would be enough); .. and the third point that I did not initially adress: - otherwise, then we need to generate as many mxs-bootlets as there are DTS configured, but I did not suggest that; you are probably better placed to answer that question than I am! ;-) Your new patch does address that part quite nicely, if that makes sense to have. The next step depends on which of the above three scenarios you believe is the best. Thank you! Regards, Yann E. MORIN. > Best regards, > > Laurent > > --- > boot/mxs-bootlets/mxs-bootlets.mk | 40 +++++++++++++++++++++++-------- > 1 file changed, 30 insertions(+), 10 deletions(-) > > diff --git a/boot/mxs-bootlets/mxs-bootlets.mk b/boot/mxs-bootlets/mxs-bootlets.mk > index adc22767..0a1f7009 100644 > --- a/boot/mxs-bootlets/mxs-bootlets.mk > +++ b/boot/mxs-bootlets/mxs-bootlets.mk > @@ -41,17 +41,17 @@ MXS_BOOTLETS_LICENSE = GPL-2.0+ > ifeq ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) > MXS_BOOTLETS_DEPENDENCIES += barebox > MXS_BOOTLETS_BOOTDESC = barebox$(MXS_BOOTLETS_IVT_SUFFIX).bd > -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX).sb > +MXS_BOOTLETS_BOOTSTREAM_BASE = $(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX) > > else ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) > MXS_BOOTLETS_DEPENDENCIES += linux > MXS_BOOTLETS_BOOTDESC = linux$(MXS_BOOTLETS_IVT_SUFFIX).bd > -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX).sb > +MXS_BOOTLETS_BOOTSTREAM_BASE = $(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX) > > else ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) > MXS_BOOTLETS_DEPENDENCIES += uboot > MXS_BOOTLETS_BOOTDESC = uboot$(MXS_BOOTLETS_IVT_SUFFIX).bd > -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX).sb > +MXS_BOOTLETS_BOOTSTREAM_BASE = $(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX) > endif > > ifeq ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) > @@ -65,11 +65,16 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > $(MAKE1) -C $(@D) linux_prep > endef > + > define MXS_BOOTLETS_SED_LINUX > sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > + sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage$(1)";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > endef > + > +ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB), y) > +MXS_BOOTLETS_DTS_NAMES = $(foreach dts_name,$(LINUX_DTS_NAME),.$(strip $(dts_name))) > endif > +endif #BR2_TARGET_MXS_BOOTLETS_LINUX > > ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) > define MXS_BOOTLETS_SED_UBOOT > @@ -83,7 +88,13 @@ endef > > MXS_BOOTLETS_POST_EXTRACT_HOOKS += MXS_BOOTLETS_INSTALL_BAREBOX_BOOTDESC > > -define MXS_BOOTLETS_BUILD_CMDS > +define MXS_BOOTLETS_ELFTOSB_CMD > + $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ > + -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ > + -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE)$(1).sb > +endef > + > +define MXS_BOOTLETS_BUILD_CMDS > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > $(MAKE1) -C $(@D) power_prep > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > @@ -92,15 +103,24 @@ define MXS_BOOTLETS_BUILD_CMDS > sed -i 's,[^ *]power_prep.*;,\tpower_prep="$(@D)/power_prep/power_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > sed -i 's,[^ *]sdram_prep.*;,\tsdram_prep="$(@D)/boot_prep/boot_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > $(MXS_BOOTLETS_SED_BAREBOX) > - $(MXS_BOOTLETS_SED_LINUX) > $(MXS_BOOTLETS_SED_UBOOT) > - $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ > - -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ > - -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) > + $(if $(MXS_BOOTLETS_DTS_NAMES), > + $(foreach dts_name, $(MXS_BOOTLETS_DTS_NAMES), > + $(call MXS_BOOTLETS_SED_LINUX,$(dts_name)) > + $(call MXS_BOOTLETS_ELFTOSB_CMD,$(dts_name)) > + ), > + $(call MXS_BOOTLETS_SED_LINUX) > + $(call MXS_BOOTLETS_ELFTOSB_CMD) > + ) > endef > > define MXS_BOOTLETS_INSTALL_TARGET_CMDS > - cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) $(BINARIES_DIR)/ > + $(if $(MXS_BOOTLETS_DTS_NAMES), > + $(foreach dts_name, $(MXS_BOOTLETS_DTS_NAMES), > + cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE)$(dts_name).sb $(BINARIES_DIR)/ > + ), > + cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE).sb $(BINARIES_DIR)/ > + ) > endef > > $(eval $(generic-package)) > -- > 2.17.1 > > -----Original Message----- > From: Yann E. MORIN <yann.morin.1998@free.fr> > Sent: Thursday, January 11, 2024 4:26 PM > To: Badel, Laurent <LaurentBadel@eaton.com> > Cc: buildroot@buildroot.org > Subject: [EXTERNAL] Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB > > Laurent, All, > > On 2024-01-11 13:19 +0000, Laurent Badel via buildroot spake thusly: > > When building a zImage with appended DTB, buildroot creates a copy of > > the zImage named zImage.$(LINUX_DTS_NAME). mxs-bootlets.mk does not > > take this into consideration and instead passes the original zImage > > (without DTB appended) to elftosb to generate the SB file. Thus, make > > sure that the correct zImage is used in this process. > > > > Note: this patch only supports a single DTS specified in the > > configuration, because there is no obvious use case for multiple DTS's > > with mxs-bootlets. If multiple DTS's are configured, only the first > > one will be used. > > That's a bit unfortunate that we have to rely on the ordering. > > So, if using two or more DTS in cunjunction with mxs-bootlets does not make sense, should that be forbidden? I.e. would we need something like the following (code slightly elided for brevity): > > ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) > ifeq ($(BR2_BOOT_MXS_BOOTLETS),y) > ifneq ($(words $(LINUX_DTS_NAME)),1) > $(error More than one DTS specified; mxs-bootlets can only use one) > endif > endif > define MXS_BOOTLETS_BUILD_LINUX_PREP > ... > endef > MXS_BOOTLETS_ZIMAGE_NAME = zImage$(....) > define MXS_BOOTLETS_SED_LINUX > ... > endif > endif > > (See below the name of the variable.) > > > Signed-off-by: Laurent Badel <laurentbadel@eaton.com> > > --- > > boot/mxs-bootlets/mxs-bootlets.mk | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/boot/mxs-bootlets/mxs-bootlets.mk > > b/boot/mxs-bootlets/mxs-bootlets.mk > > index adc22767..2003b23f 100644 > > --- a/boot/mxs-bootlets/mxs-bootlets.mk > > +++ b/boot/mxs-bootlets/mxs-bootlets.mk > > @@ -65,9 +65,10 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP > > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > > $(MAKE1) -C $(@D) linux_prep endef > > +ZIMAGE_NAME=zImage$(if $(BR2_LINUX_KERNEL_APPENDED_DTB),.$(notdir > > +$(firstword $(LINUX_DTS_NAME)))) > > The namespace is global, so the variable names have to be manually scopped with the name of the package they apply to, in this case: > > MXS_BOOTLETS_ZIMAGE_NAME > > Regards, > Yann E. MORIN. > > > define MXS_BOOTLETS_SED_LINUX > > sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > > - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > > + sed -i 's,[^ > > + *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/$(ZIMAGE_NAME)";,' > > + $(@D)/$(MXS_BOOTLETS_BOOTDESC) > > endef > > endif > > > > -- > > 2.17.1 > > > > _______________________________________________ > > buildroot mailing list > > buildroot@buildroot.org > > https://list/ > > s.buildroot.org%2Fmailman%2Flistinfo%2Fbuildroot&data=05%7C02%7Clauren > > tbadel%40eaton.com%7C28583d4b11284523498408dc12b9a4b4%7Cd6525c95b90643 > > 1ab926e9b51ba43cc4%7C1%7C0%7C638405835765391645%7CUnknown%7CTWFpbGZsb3 > > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7 > > C3000%7C%7C%7C&sdata=koINDlCH%2Bo4Tv20L6jeEvTllYPrpESfnGcFRL%2F%2BbKu0 > > %3D&reserved=0 > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' > | conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > '------------------------------^-------^------------------^--------------------' -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB 2024-01-12 16:21 ` Yann E. MORIN @ 2024-01-16 9:32 ` Badel, Laurent via buildroot 0 siblings, 0 replies; 6+ messages in thread From: Badel, Laurent via buildroot @ 2024-01-16 9:32 UTC (permalink / raw) To: Yann E. MORIN; +Cc: buildroot@buildroot.org Hi Yann, Thanks a lot for your comments again, I've found what I think is a nice improvement over my last proposal, so I will submit a new clean patch as I don't want to take any more of your time unnecessarily. > I've quickly tried a few tricks involving the use of $(space) and/or > $(empty), but none did work... My apologies, I didn't mean to have you expend effort on this. > The next step depends on which of the above three scenarios you > believe is the best. To be honest, I am just having trouble imagining why one would want to use multiple DTS in this case, but I might very well be missing some use case. I do think however, that if there are multiple DTS configured and they are appended to the zImage, then the user would expect several SB files to be produced in the output. I have to admit that my initial attempt was a bit sloppy in that regard and I think that this new patch will address the issue much better. I have actually had a small in-house patch to fix this problem for at least 3 years now, and we only decided to submit a patch recently while revisiting this part of the code (in retrospect, I'm sorry that we did not report the issue immediately, that would have been better). The fact that no one seems to have complained about the issue during this time, probably suggests that there is not too much demand for this type of configuration and that drove my initial idea to go for the simplest solution - but that's not nearly always the best choice. Last but not least, thank you, and everyone, for your efforts building/maintaining buildroot. It is a great project. Best regards, Laurent -----Original Message----- From: Yann E. MORIN <yann.morin.1998@free.fr> Sent: Friday, January 12, 2024 5:21 PM To: Badel, Laurent <LaurentBadel@eaton.com> Cc: buildroot@buildroot.org Subject: [EXTERNAL] Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB Laurent, All, On 2024-01-12 08:30 +0000, Badel, Laurent spake thusly: > Thank you very much for your time reviewing/commenting on the patch. > I spent some time trying to find a way to support multiple DTSs for > the sake of completeness, please see the patch below. After a cursory look, it looks quite OK. > I would have preferred to use a single loop for all cases but couldn't > find a good way to iterate with an empty string, so this results in > slight duplication of code unfortunately. Err... I don't have a much better idea either... We could cheat with with some quoting tricks, like adding an item in the list that is just a pair of sngle quotes, which for the _SED macro would close and reopen the sed expression, and for the _ELF_TOSB would be interpreted just as an empty string. That's a dirt trick, though, and quite a bit fragile. I.e. somethinglike (nopte also the use of patsubst instead of foreach): # '' is used to create an empty item: # - closes and reopens the single-quoted string of the sed expression # - opens and closes an empty string in the elftosb output filename MXS_BOOTLETS_DTS_NAMES = '' $(patsubst %,.%,$(LINUX_DTS_NAME)) I'm not sold on the above, because, yes, it's fragile... I've quickly tried a few tricks involving the use of $(space) and/or $(empty), but none did work... > I have tested building mxs-bootlets with the following setups: > 1. zImage with appended DTB, two DTSs configured (one in-tree, the > other out-of-tree): > -> generates 2 SB files with extension .$(dtb_name).sb 2. zImage, > no DTS: > -> generates a single SB file with the .sb extension. > 3. zImage, two DTSs as in (1) but not appended: > -> generates a single SB file with the .sb extension Cool, thanks for the effort in looking into, and in testing, that! > I checked the output shell commands and they looked as expected, too. > I also minded your comment regarding the naming of variables, thank > you for pointing this out. > > If you think that is a better solution, I am happy to re-submit a > clean patch. Well, just looking at the code, it looks better, because we do not rely on the ordering of the DTS names to generate one mxs-bootlet, aa we get as many mxs-bootlets as there are configured DTS. However, I have no idea if it makes sense to have as many bootlets as there are DTS names; you are better placed to know that. Sorry if my review seemed misled you into this change. What my point in my review was, was two fold: - _if_ it does not make sense to have multiple DTS configured when using mxs-bootlets, then the situation must be caught and the user must be warned about that (hence my code snippet to detect the case); - otherwise, _if_ it is OK to have multiple DTS configured, and that it is OK to build a single mxs-bootlet in that case, then it is sad to rely on the ordering (but I did not provide any suggestion to fix that, sorry; maybe just a comment stating so in the help text for BR2_TARGET_MXS_BOOTLETS_LINUX would be enough); .. and the third point that I did not initially adress: - otherwise, then we need to generate as many mxs-bootlets as there are DTS configured, but I did not suggest that; you are probably better placed to answer that question than I am! ;-) Your new patch does address that part quite nicely, if that makes sense to have. The next step depends on which of the above three scenarios you believe is the best. Thank you! Regards, Yann E. MORIN. > Best regards, > > Laurent > > --- > boot/mxs-bootlets/mxs-bootlets.mk | 40 > +++++++++++++++++++++++-------- > 1 file changed, 30 insertions(+), 10 deletions(-) > > diff --git a/boot/mxs-bootlets/mxs-bootlets.mk > b/boot/mxs-bootlets/mxs-bootlets.mk > index adc22767..0a1f7009 100644 > --- a/boot/mxs-bootlets/mxs-bootlets.mk > +++ b/boot/mxs-bootlets/mxs-bootlets.mk > @@ -41,17 +41,17 @@ MXS_BOOTLETS_LICENSE = GPL-2.0+ ifeq > ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) > MXS_BOOTLETS_DEPENDENCIES += barebox > MXS_BOOTLETS_BOOTDESC = barebox$(MXS_BOOTLETS_IVT_SUFFIX).bd > -MXS_BOOTLETS_BOOTSTREAM = > $(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX).sb > +MXS_BOOTLETS_BOOTSTREAM_BASE = > +$(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX) > > else ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) > MXS_BOOTLETS_DEPENDENCIES += linux > MXS_BOOTLETS_BOOTDESC = linux$(MXS_BOOTLETS_IVT_SUFFIX).bd > -MXS_BOOTLETS_BOOTSTREAM = > $(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX).sb > +MXS_BOOTLETS_BOOTSTREAM_BASE = > +$(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX) > > else ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) > MXS_BOOTLETS_DEPENDENCIES += uboot > MXS_BOOTLETS_BOOTDESC = uboot$(MXS_BOOTLETS_IVT_SUFFIX).bd > -MXS_BOOTLETS_BOOTSTREAM = > $(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX).sb > +MXS_BOOTLETS_BOOTSTREAM_BASE = > +$(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX) > endif > > ifeq ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) > @@ -65,11 +65,16 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > $(MAKE1) -C $(@D) linux_prep endef > + > define MXS_BOOTLETS_SED_LINUX > sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > + sed -i 's,[^ > + *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage$(1)";,' > + $(@D)/$(MXS_BOOTLETS_BOOTDESC) > endef > + > +ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB), y) MXS_BOOTLETS_DTS_NAMES = > +$(foreach dts_name,$(LINUX_DTS_NAME),.$(strip $(dts_name))) > endif > +endif #BR2_TARGET_MXS_BOOTLETS_LINUX > > ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) > define MXS_BOOTLETS_SED_UBOOT > @@ -83,7 +88,13 @@ endef > > MXS_BOOTLETS_POST_EXTRACT_HOOKS += > MXS_BOOTLETS_INSTALL_BAREBOX_BOOTDESC > > -define MXS_BOOTLETS_BUILD_CMDS > +define MXS_BOOTLETS_ELFTOSB_CMD > + $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ > + -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ > + -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE)$(1).sb > +endef > + > +define MXS_BOOTLETS_BUILD_CMDS > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > $(MAKE1) -C $(@D) power_prep > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > @@ -92,15 +103,24 @@ define MXS_BOOTLETS_BUILD_CMDS > sed -i 's,[^ *]power_prep.*;,\tpower_prep="$(@D)/power_prep/power_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > sed -i 's,[^ *]sdram_prep.*;,\tsdram_prep="$(@D)/boot_prep/boot_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > $(MXS_BOOTLETS_SED_BAREBOX) > - $(MXS_BOOTLETS_SED_LINUX) > $(MXS_BOOTLETS_SED_UBOOT) > - $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ > - -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ > - -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) > + $(if $(MXS_BOOTLETS_DTS_NAMES), > + $(foreach dts_name, $(MXS_BOOTLETS_DTS_NAMES), > + $(call MXS_BOOTLETS_SED_LINUX,$(dts_name)) > + $(call MXS_BOOTLETS_ELFTOSB_CMD,$(dts_name)) > + ), > + $(call MXS_BOOTLETS_SED_LINUX) > + $(call MXS_BOOTLETS_ELFTOSB_CMD) > + ) > endef > > define MXS_BOOTLETS_INSTALL_TARGET_CMDS > - cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) $(BINARIES_DIR)/ > + $(if $(MXS_BOOTLETS_DTS_NAMES), > + $(foreach dts_name, $(MXS_BOOTLETS_DTS_NAMES), > + cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE)$(dts_name).sb $(BINARIES_DIR)/ > + ), > + cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASE).sb $(BINARIES_DIR)/ > + ) > endef > > $(eval $(generic-package)) > -- > 2.17.1 > > -----Original Message----- > From: Yann E. MORIN <yann.morin.1998@free.fr> > Sent: Thursday, January 11, 2024 4:26 PM > To: Badel, Laurent <LaurentBadel@eaton.com> > Cc: buildroot@buildroot.org > Subject: [EXTERNAL] Re: [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add > support for zImage with appended DTB > > Laurent, All, > > On 2024-01-11 13:19 +0000, Laurent Badel via buildroot spake thusly: > > When building a zImage with appended DTB, buildroot creates a copy > > of the zImage named zImage.$(LINUX_DTS_NAME). mxs-bootlets.mk does > > not take this into consideration and instead passes the original > > zImage (without DTB appended) to elftosb to generate the SB file. > > Thus, make sure that the correct zImage is used in this process. > > > > Note: this patch only supports a single DTS specified in the > > configuration, because there is no obvious use case for multiple > > DTS's with mxs-bootlets. If multiple DTS's are configured, only the > > first one will be used. > > That's a bit unfortunate that we have to rely on the ordering. > > So, if using two or more DTS in cunjunction with mxs-bootlets does not make sense, should that be forbidden? I.e. would we need something like the following (code slightly elided for brevity): > > ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) > ifeq ($(BR2_BOOT_MXS_BOOTLETS),y) > ifneq ($(words $(LINUX_DTS_NAME)),1) > $(error More than one DTS specified; mxs-bootlets can only use one) > endif > endif > define MXS_BOOTLETS_BUILD_LINUX_PREP > ... > endef > MXS_BOOTLETS_ZIMAGE_NAME = zImage$(....) > define MXS_BOOTLETS_SED_LINUX > ... > endif > endif > > (See below the name of the variable.) > > > Signed-off-by: Laurent Badel <laurentbadel@eaton.com> > > --- > > boot/mxs-bootlets/mxs-bootlets.mk | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/boot/mxs-bootlets/mxs-bootlets.mk > > b/boot/mxs-bootlets/mxs-bootlets.mk > > index adc22767..2003b23f 100644 > > --- a/boot/mxs-bootlets/mxs-bootlets.mk > > +++ b/boot/mxs-bootlets/mxs-bootlets.mk > > @@ -65,9 +65,10 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP > > BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ > > $(MAKE1) -C $(@D) linux_prep endef > > +ZIMAGE_NAME=zImage$(if $(BR2_LINUX_KERNEL_APPENDED_DTB),.$(notdir > > +$(firstword $(LINUX_DTS_NAME)))) > > The namespace is global, so the variable names have to be manually scopped with the name of the package they apply to, in this case: > > MXS_BOOTLETS_ZIMAGE_NAME > > Regards, > Yann E. MORIN. > > > define MXS_BOOTLETS_SED_LINUX > > sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > > - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) > > + sed -i 's,[^ > > + *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/$(ZIMAGE_NAME)";,' > > + $(@D)/$(MXS_BOOTLETS_BOOTDESC) > > endef > > endif > > > > -- > > 2.17.1 > > > > _______________________________________________ > > buildroot mailing list > > buildroot@buildroot.org > > https://list/ > > s.buildroot.org%2Fmailman%2Flistinfo%2Fbuildroot&data=05%7C02%7Claur > > en > > tbadel%40eaton.com%7C28583d4b11284523498408dc12b9a4b4%7Cd6525c95b906 > > 43 > > 1ab926e9b51ba43cc4%7C1%7C0%7C638405835765391645%7CUnknown%7CTWFpbGZs > > b3 > > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D > > %7 > > C3000%7C%7C%7C&sdata=koINDlCH%2Bo4Tv20L6jeEvTllYPrpESfnGcFRL%2F%2BbK > > u0 > > %3D&reserved=0 > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' > | conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > '------------------------------^-------^------------------^--------------------' -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' | conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB @ 2024-01-16 9:34 Laurent Badel via buildroot 0 siblings, 0 replies; 6+ messages in thread From: Laurent Badel via buildroot @ 2024-01-16 9:34 UTC (permalink / raw) To: buildroot; +Cc: Laurent Badel When building a zImage with appended DTBs, buildroot creates copies of the zImage named zImage.$(LINUX_DTS_NAME). mxs-bootlets.mk does not take this into consideration and passes only the original zImage (without DTB appended) to elftosb to generate the SB file. Thus, make sure that the correct zImage files are used in this process. Signed-off-by: Laurent Badel <laurentbadel@eaton.com> --- boot/mxs-bootlets/mxs-bootlets.mk | 36 +++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/boot/mxs-bootlets/mxs-bootlets.mk b/boot/mxs-bootlets/mxs-bootlets.mk index adc22767..efcee16a 100644 --- a/boot/mxs-bootlets/mxs-bootlets.mk +++ b/boot/mxs-bootlets/mxs-bootlets.mk @@ -38,20 +38,21 @@ endif MXS_BOOTLETS_DEPENDENCIES = host-elftosb MXS_BOOTLETS_LICENSE = GPL-2.0+ +MXS_BOOTLETS_BOOTSTREAM_EXTS = .sb ifeq ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) MXS_BOOTLETS_DEPENDENCIES += barebox MXS_BOOTLETS_BOOTDESC = barebox$(MXS_BOOTLETS_IVT_SUFFIX).bd -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX).sb +MXS_BOOTLETS_BOOTSTREAM_BASENAME = $(MXS_BOOTLETS_BOARD)_barebox$(MXS_BOOTLETS_IVT_SUFFIX) else ifeq ($(BR2_TARGET_MXS_BOOTLETS_LINUX),y) MXS_BOOTLETS_DEPENDENCIES += linux MXS_BOOTLETS_BOOTDESC = linux$(MXS_BOOTLETS_IVT_SUFFIX).bd -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX).sb +MXS_BOOTLETS_BOOTSTREAM_BASENAME = $(MXS_BOOTLETS_BOARD)_linux$(MXS_BOOTLETS_IVT_SUFFIX) else ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) MXS_BOOTLETS_DEPENDENCIES += uboot MXS_BOOTLETS_BOOTDESC = uboot$(MXS_BOOTLETS_IVT_SUFFIX).bd -MXS_BOOTLETS_BOOTSTREAM = $(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX).sb +MXS_BOOTLETS_BOOTSTREAM_BASENAME = $(MXS_BOOTLETS_BOARD)_uboot$(MXS_BOOTLETS_IVT_SUFFIX) endif ifeq ($(BR2_TARGET_MXS_BOOTLETS_BAREBOX),y) @@ -65,10 +66,18 @@ define MXS_BOOTLETS_BUILD_LINUX_PREP BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ $(MAKE1) -C $(@D) linux_prep endef + define MXS_BOOTLETS_SED_LINUX sed -i 's,[^ *]linux_prep.*;,\tlinux_prep="$(@D)/linux_prep/output-target/linux_prep";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) - sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) endef + +define MXS_BOOTLETS_SED_ZIMAGE + sed -i 's,[^ *]zImage.*;,\tzImage="$(LINUX_DIR)/arch/arm/boot/zImage$(1)";,' $(@D)/$(MXS_BOOTLETS_BOOTDESC) +endef + +ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB), y) + MXS_BOOTLETS_BOOTSTREAM_EXTS = $(foreach dts_name,$(LINUX_DTS_NAME),.$(strip $(dts_name)).sb) +endif endif ifeq ($(BR2_TARGET_MXS_BOOTLETS_UBOOT),y) @@ -83,7 +92,13 @@ endef MXS_BOOTLETS_POST_EXTRACT_HOOKS += MXS_BOOTLETS_INSTALL_BAREBOX_BOOTDESC -define MXS_BOOTLETS_BUILD_CMDS +define MXS_BOOTLETS_ELFTOSB_CMD + $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ + -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ + -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASENAME)$(1) +endef + +define MXS_BOOTLETS_BUILD_CMDS BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ $(MAKE1) -C $(@D) power_prep BOARD=$(MXS_BOOTLETS_BOARD) CROSS_COMPILE="$(TARGET_CROSS)" \ @@ -94,13 +109,16 @@ define MXS_BOOTLETS_BUILD_CMDS $(MXS_BOOTLETS_SED_BAREBOX) $(MXS_BOOTLETS_SED_LINUX) $(MXS_BOOTLETS_SED_UBOOT) - $(HOST_DIR)/bin/elftosb $(MXS_BOOTLETS_ELFTOSB_OPTIONS) \ - -z -c $(@D)/$(MXS_BOOTLETS_BOOTDESC) \ - -o $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) + $(foreach ext, $(MXS_BOOTLETS_BOOTSTREAM_EXTS), + $(call MXS_BOOTLETS_SED_ZIMAGE,$(subst .sb,,$(ext))) + $(call MXS_BOOTLETS_ELFTOSB_CMD,$(ext)) + ) endef define MXS_BOOTLETS_INSTALL_TARGET_CMDS - cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM) $(BINARIES_DIR)/ + $(foreach ext, $(MXS_BOOTLETS_BOOTSTREAM_EXTS), + cp $(@D)/$(MXS_BOOTLETS_BOOTSTREAM_BASENAME)$(ext) $(BINARIES_DIR)/ + ) endef $(eval $(generic-package)) -- 2.17.1 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-16 9:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-11 13:19 [Buildroot] [PATCH 1/1] boot/mxs-bootlets: add support for zImage with appended DTB Laurent Badel via buildroot 2024-01-11 15:26 ` Yann E. MORIN 2024-01-12 8:30 ` Badel, Laurent via buildroot 2024-01-12 16:21 ` Yann E. MORIN 2024-01-16 9:32 ` Badel, Laurent via buildroot -- strict thread matches above, loose matches on Subject: below -- 2024-01-16 9:34 Laurent Badel via buildroot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox