* [Buildroot] [PATCH 1/1] boot/uboot/uboot.mk: fix zynqmp without pmufw @ 2022-11-21 20:16 Brandon Maier via buildroot 2022-11-21 22:13 ` Luca Ceresoli via buildroot 0 siblings, 1 reply; 3+ messages in thread From: Brandon Maier via buildroot @ 2022-11-21 20:16 UTC (permalink / raw) To: buildroot; +Cc: Luca Ceresoli, Brandon Maier, Neal Frager Commit d07e6b70 (boot/uboot/uboot.mk: add pmufw.elf support) broke configurations where the UBOOT_ZYNQMP_PMUFW was blank. Previously it would set the U-Boot CONFIG_PMUFW_INIT_FILE to the blank string, but now it will set it to ".bin" which causes U-Boot to fail to build. To fix, disable all the pmufw code if the UBOOT_ZYNQMP_PMUFW is blank. Signed-off-by: Brandon Maier <brandon.maier@collins.com> --- boot/uboot/uboot.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk index 0439ec5e4b..b86248e1f7 100644 --- a/boot/uboot/uboot.mk +++ b/boot/uboot/uboot.mk @@ -383,6 +383,7 @@ endef ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y) UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW)) +ifneq ($(UBOOT_ZYNQMP_PMUFW),) ifneq ($(findstring ://,$(UBOOT_ZYNQMP_PMUFW)),) UBOOT_EXTRA_DOWNLOADS += $(UBOOT_ZYNQMP_PMUFW) @@ -398,6 +399,7 @@ define UBOOT_ZYNQMP_KCONFIG_PMUFW objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin) $(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_BASENAME).bin") endef +endif # UBOOT_ZYNQMP_PMUFW UBOOT_ZYNQMP_PM_CFG = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PM_CFG)) ifneq ($(UBOOT_ZYNQMP_PM_CFG),) -- 2.38.1 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Buildroot] [PATCH 1/1] boot/uboot/uboot.mk: fix zynqmp without pmufw 2022-11-21 20:16 [Buildroot] [PATCH 1/1] boot/uboot/uboot.mk: fix zynqmp without pmufw Brandon Maier via buildroot @ 2022-11-21 22:13 ` Luca Ceresoli via buildroot 2022-11-22 5:55 ` Frager, Neal via buildroot 0 siblings, 1 reply; 3+ messages in thread From: Luca Ceresoli via buildroot @ 2022-11-21 22:13 UTC (permalink / raw) To: Brandon Maier; +Cc: Neal Frager, buildroot Hi Brandon, On Mon, 21 Nov 2022 14:16:38 -0600 Brandon Maier <brandon.maier@collins.com> wrote: > Commit d07e6b70 (boot/uboot/uboot.mk: add pmufw.elf support) broke > configurations where the UBOOT_ZYNQMP_PMUFW was blank. Previously it > would set the U-Boot CONFIG_PMUFW_INIT_FILE to the blank string, but now > it will set it to ".bin" which causes U-Boot to fail to build. > > To fix, disable all the pmufw code if the UBOOT_ZYNQMP_PMUFW is blank. > > Signed-off-by: Brandon Maier <brandon.maier@collins.com> You are right, this was missed during patch review. Thanks for reporting! > diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk > index 0439ec5e4b..b86248e1f7 100644 > --- a/boot/uboot/uboot.mk > +++ b/boot/uboot/uboot.mk > @@ -383,6 +383,7 @@ endef > ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y) > > UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW)) > +ifneq ($(UBOOT_ZYNQMP_PMUFW),) > I don't like an empty line after the ifneq unless we have one before _and_ one before the endif too. But this is minor, stylish nitpicking, see below for something a bit more interesting. > ifneq ($(findstring ://,$(UBOOT_ZYNQMP_PMUFW)),) > UBOOT_EXTRA_DOWNLOADS += $(UBOOT_ZYNQMP_PMUFW) > @@ -398,6 +399,7 @@ define UBOOT_ZYNQMP_KCONFIG_PMUFW > objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin) > $(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_BASENAME).bin") > endef > +endif # UBOOT_ZYNQMP_PMUFW > > UBOOT_ZYNQMP_PM_CFG = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PM_CFG)) > ifneq ($(UBOOT_ZYNQMP_PM_CFG),) This would be an effective fix for the known cases (empty string, .elf, .bin), but it would break again whenever Xilinx or anybody else invents another extension. Not likely maybe, but overall I would prefer a fix like (untested): define UBOOT_ZYNQMP_KCONFIG_PMUFW + $(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_PATH)") $(if $(filter %.elf,$(UBOOT_ZYNQMP_PMUFW_PATH)), - objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin) + objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin + $(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_BASENAME).bin"))) endef The above reverts to always setting CONFIG_PMUFW_INIT_FILE to the unmodified value of UBOOT_ZYNQMP_PMUFW_PATH as we used to do before d07e6b70, then mangles it only in the special "*.elf" case. Would you mind testing that and sending a patch? -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Buildroot] [PATCH 1/1] boot/uboot/uboot.mk: fix zynqmp without pmufw 2022-11-21 22:13 ` Luca Ceresoli via buildroot @ 2022-11-22 5:55 ` Frager, Neal via buildroot 0 siblings, 0 replies; 3+ messages in thread From: Frager, Neal via buildroot @ 2022-11-22 5:55 UTC (permalink / raw) To: Luca Ceresoli, Brandon Maier; +Cc: buildroot@buildroot.org [-- Attachment #1: Type: text/plain, Size: 2998 bytes --] Hi Brandon, > Commit d07e6b70 (boot/uboot/uboot.mk: add pmufw.elf support) broke > configurations where the UBOOT_ZYNQMP_PMUFW was blank. Previously it > would set the U-Boot CONFIG_PMUFW_INIT_FILE to the blank string, but > now it will set it to ".bin" which causes U-Boot to fail to build. > > To fix, disable all the pmufw code if the UBOOT_ZYNQMP_PMUFW is blank. > > Signed-off-by: Brandon Maier <brandon.maier@collins.com> > You are right, this was missed during patch review. Thanks for reporting! > diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk index > 0439ec5e4b..b86248e1f7 100644 > --- a/boot/uboot/uboot.mk > +++ b/boot/uboot/uboot.mk > @@ -383,6 +383,7 @@ endef > ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y) > > UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW)) > +ifneq ($(UBOOT_ZYNQMP_PMUFW),) > > I don't like an empty line after the ifneq unless we have one before _and_ one before the endif too. But this is minor, stylish nitpicking, see below for something a bit more interesting. > ifneq ($(findstring ://,$(UBOOT_ZYNQMP_PMUFW)),) > UBOOT_EXTRA_DOWNLOADS += $(UBOOT_ZYNQMP_PMUFW) @@ -398,6 +399,7 @@ > define UBOOT_ZYNQMP_KCONFIG_PMUFW > objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin) > $(call > KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_BASENAME) > .bin") > endef > +endif # UBOOT_ZYNQMP_PMUFW > > UBOOT_ZYNQMP_PM_CFG = $(call > qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PM_CFG)) > ifneq ($(UBOOT_ZYNQMP_PM_CFG),) > This would be an effective fix for the known cases (empty string, .elf, .bin), but it would break again whenever Xilinx or anybody else invents another extension. Not likely maybe, but overall I would prefer a fix like (untested): > define UBOOT_ZYNQMP_KCONFIG_PMUFW > + $(call > + KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_PATH)") > $(if $(filter %.elf,$(UBOOT_ZYNQMP_PMUFW_PATH)), > - objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin) > + objcopy -O binary -I elf32-little $(UBOOT_ZYNQMP_PMUFW_BASENAME).elf $(UBOOT_ZYNQMP_PMUFW_BASENAME).bin > + $(call > + KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"$(UBOOT_ZYNQMP_PMUFW_BASENAME) > + .bin"))) > endef > The above reverts to always setting CONFIG_PMUFW_INIT_FILE to the unmodified value of UBOOT_ZYNQMP_PMUFW_PATH as we used to do before d07e6b70, then mangles it only in the special "*.elf" case. Thank you for catching this issue. I agree with Luca that this would be a better solution to keep the modification specific to only the special "*.elf" case. I have nothing further to offer as Luca already provided a very complete review. I will mark this patch as reviewed when Luca's feedback has been taken into account. Thank you for your support! Best regards, Neal Frager AMD [-- Attachment #2: winmail.dat --] [-- Type: application/ms-tnef, Size: 15006 bytes --] [-- Attachment #3: Type: text/plain, Size: 150 bytes --] _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-22 5:55 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-21 20:16 [Buildroot] [PATCH 1/1] boot/uboot/uboot.mk: fix zynqmp without pmufw Brandon Maier via buildroot 2022-11-21 22:13 ` Luca Ceresoli via buildroot 2022-11-22 5:55 ` Frager, Neal via buildroot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox