All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
@ 2023-03-26 14:09 Neal Frager via buildroot
  2023-03-26 14:09 ` [Buildroot] [PATCH v1 2/2] boot/uboot.mk: new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE Neal Frager via buildroot
  2023-03-26 20:05 ` [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Thomas Petazzoni via buildroot
  0 siblings, 2 replies; 14+ messages in thread
From: Neal Frager via buildroot @ 2023-03-26 14:09 UTC (permalink / raw)
  To: buildroot
  Cc: ibai.erkiaga-elorza, luca.ceresoli, thomas.petazzoni,
	Neal Frager, michal.simek

This patch adds a new package to buildroot for building the zynqmp pmufw
with the requirement that the user must provide an external microblaze
toolchain.

The below example config options can be used to build the pmufw.elf v2022.2
using the Xilinx toolchain.

BR2_PACKAGE_ZYNQMP_FIRMWARE=y
BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION="v2022.2"
BR2_PACKAGE_ZYNQMP_FIRMWARE_TOOLCHAIN_PATH="/opt/Xilinx/Vitis/2022.2/gnu/microblaze/lin"
BR2_PACKAGE_ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX="microblaze-xilinx-elf-"

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 DEVELOPERS                                    |  1 +
 package/Config.in                             |  1 +
 package/zynqmp-firmware/Config.in             | 32 ++++++++++++++++
 ...misc-Makefile-build-par_libs-with-j1.patch | 31 +++++++++++++++
 package/zynqmp-firmware/zynqmp-firmware.mk    | 38 +++++++++++++++++++
 5 files changed, 103 insertions(+)
 create mode 100644 package/zynqmp-firmware/Config.in
 create mode 100644 package/zynqmp-firmware/v1-0001-pmufw-misc-Makefile-build-par_libs-with-j1.patch
 create mode 100644 package/zynqmp-firmware/zynqmp-firmware.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index b6d288c54f..d3436d40f2 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -2151,6 +2151,7 @@ F:	configs/zynqmp_zcu106_defconfig
 F:	configs/zynqmp_kria_kv260_defconfig
 F:	package/bootgen/
 F:	package/versal-firmware/
+F:	package/zynqmp-firmware/
 
 N:	Nicola Di Lieto <nicola.dilieto@gmail.com>
 F:	package/uacme/
diff --git a/package/Config.in b/package/Config.in
index 0f8dab3e71..6c7419490d 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -450,6 +450,7 @@ menu "Firmware"
 	source "package/wilc-firmware/Config.in"
 	source "package/wilink-bt-firmware/Config.in"
 	source "package/zd1211-firmware/Config.in"
+	source "package/zynqmp-firmware/Config.in"
 endmenu
 	source "package/18xx-ti-utils/Config.in"
 	source "package/a10disp/Config.in"
diff --git a/package/zynqmp-firmware/Config.in b/package/zynqmp-firmware/Config.in
new file mode 100644
index 0000000000..3d0d0605f6
--- /dev/null
+++ b/package/zynqmp-firmware/Config.in
@@ -0,0 +1,32 @@
+config BR2_PACKAGE_ZYNQMP_FIRMWARE
+	bool "zynqmp-firmware"
+	help
+	  This package builds the PMU Firmware application required to run
+	  U-Boot and Linux in the Zynq MPSoC devices.
+
+if BR2_PACKAGE_ZYNQMP_FIRMWARE
+
+config BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION
+	string "firmware version"
+	default "v2022.2"
+	help
+	  Release version of zynqmp firmware.
+
+config BR2_PACKAGE_ZYNQMP_FIRMWARE_TOOLCHAIN_PATH
+	string "toolchain path"
+	default "/opt/Xilinx/Vitis/2022.2/gnu/microblaze/lin"
+	help
+	  Path to pre-installed microblaze toolchain.
+
+config BR2_PACKAGE_ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX
+	string "toolchain prefix"
+	default "microblaze-xilinx-elf-"
+	help
+	  Pre-installed microblaze toolchain prefix.
+
+config BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26
+	bool "kria-k26"
+	help
+	  Adds additional CFLAGS for Kria K26 SOMs.
+
+endif # BR2_PACKAGE_ZYNQMP_FIRMWARE
diff --git a/package/zynqmp-firmware/v1-0001-pmufw-misc-Makefile-build-par_libs-with-j1.patch b/package/zynqmp-firmware/v1-0001-pmufw-misc-Makefile-build-par_libs-with-j1.patch
new file mode 100644
index 0000000000..adf866637b
--- /dev/null
+++ b/package/zynqmp-firmware/v1-0001-pmufw-misc-Makefile-build-par_libs-with-j1.patch
@@ -0,0 +1,31 @@
+From 4938b250982d440fec3a1bb838be10d9a73949aa Mon Sep 17 00:00:00 2001
+From: Neal Frager <neal.frager@amd.com>
+Date: Sun, 26 Mar 2023 13:38:02 +0100
+Subject: [PATCH v1 1/1] pmufw: misc/Makefile: build par_libs with -j1
+
+This patch fixes a build problem with the pmufw par_libs.  If the -j1
+make option is not used, then building the par_libs can fail when
+building on certain servers.  Using the -j1 option guarantees build
+success of the zynqmp_pmufw.
+
+Signed-off-by: Neal Frager <neal.frager@amd.com>
+---
+ lib/sw_apps/zynqmp_pmufw/misc/Makefile | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/lib/sw_apps/zynqmp_pmufw/misc/Makefile b/lib/sw_apps/zynqmp_pmufw/misc/Makefile
+index a773498512..7f4d5540b0 100644
+--- a/lib/sw_apps/zynqmp_pmufw/misc/Makefile
++++ b/lib/sw_apps/zynqmp_pmufw/misc/Makefile
+@@ -17,7 +17,7 @@ endif
+ 
+ all:
+ 	$(MAKE) --no-print-directory seq_libs
+-	$(MAKE) -j --no-print-directory par_libs
++	$(MAKE) -j1 --no-print-directory par_libs
+ 	$(MAKE) --no-print-directory archive
+ 	@echo 'Finished building libraries'
+ 
+-- 
+2.17.1
+
diff --git a/package/zynqmp-firmware/zynqmp-firmware.mk b/package/zynqmp-firmware/zynqmp-firmware.mk
new file mode 100644
index 0000000000..3d33142258
--- /dev/null
+++ b/package/zynqmp-firmware/zynqmp-firmware.mk
@@ -0,0 +1,38 @@
+################################################################################
+#
+# zynqmp-firmware
+#
+################################################################################
+
+ZYNQMP_FIRMWARE_VERSION = $(call qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION))
+ZYNQMP_FIRMWARE_SOURCE = xilinx_$(ZYNQMP_FIRMWARE_VERSION).tar.gz
+ZYNQMP_FIRMWARE_SITE = https://github.com/Xilinx/embeddedsw/archive/refs/tags
+ZYNQMP_FIRMWARE_LICENSE = MIT
+ZYNQMP_FIRMWARE_LICENSE_FILES = license.txt
+ZYNQMP_FIRMWARE_INSTALL_IMAGES = YES
+ZYNQMP_FIRMWARE_INSTALL_TARGET = NO
+
+ZYNQMP_FIRMWARE_TOOLCHAIN_PATH = $(call qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_TOOLCHAIN_PATH))
+ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX = $(call qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX))
+
+ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
+ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects -DBOARD_SHUTDOWN_PIN=2 -DBOARD_SHUTDOWN_PIN_STATE=0 \
+		-DENABLE_EM -DENABLE_MOD_OVERTEMP -DENABLE_DYNAMIC_MIO_CONFIG -DENABLE_IOCTL \
+		-DCONNECT_PMU_GPO_2_VAL=0"
+else
+ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects"
+endif
+
+define ZYNQMP_FIRMWARE_BUILD_CMDS
+	cd $(@D)/lib/sw_apps/zynqmp_pmufw/src && make \
+		COMPILER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
+		ARCHIVER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc-ar \
+		CC=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
+		CFLAGS=$(ZYNQMP_CFLAGS)
+endef
+
+define ZYNQMP_FIRMWARE_INSTALL_IMAGES_CMDS
+	$(INSTALL) -D -m 0644 $(@D)/lib/sw_apps/zynqmp_pmufw/src/executable.elf $(BINARIES_DIR)/pmufw.elf
+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] 14+ messages in thread
* [Buildroot] [PATCH v1 2/2] boot/uboot.mk: new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE
  2023-03-26 14:09 [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Neal Frager via buildroot
@ 2023-03-26 14:09 ` Neal Frager via buildroot
  2023-03-26 20:09   ` Thomas Petazzoni via buildroot
  2023-03-26 20:05 ` [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Thomas Petazzoni via buildroot
  1 sibling, 1 reply; 14+ messages in thread
From: Neal Frager via buildroot @ 2023-03-26 14:09 UTC (permalink / raw)
  To: buildroot
  Cc: ibai.erkiaga-elorza, luca.ceresoli, thomas.petazzoni,
	Neal Frager, michal.simek

The new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE option will enable u-boot to
use the zynqmp-firmware package for building a pmufw.elf that gets included in
the generated boot.bin.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 boot/uboot/Config.in | 11 +++++++++++
 boot/uboot/uboot.mk  |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
index f5c20f5168..116df71ea3 100644
--- a/boot/uboot/Config.in
+++ b/boot/uboot/Config.in
@@ -476,6 +476,17 @@ config BR2_TARGET_UBOOT_ZYNQMP
 
 if BR2_TARGET_UBOOT_ZYNQMP
 
+config BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE
+	bool "U-Boot needs zynqmp-firmware"
+	depends on BR2_PACKAGE_ZYNQMP_FIRMWARE
+	depends on !BR2_TARGET_UBOOT_ZYNQMP_PMUFW
+	help
+	  This option instructs u-boot to build the zynqmp pmufw using
+	  the zynqmp-firmware package.  u-boot will then include this
+	  pmufw.elf in the generated boot.bin.
+
+	  This feature requires U-Boot >= 2018.07.
+
 config BR2_TARGET_UBOOT_ZYNQMP_PMUFW
 	string "PMU firmware location"
 	depends on BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG
diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
index 4eae8e95c3..78e15be453 100644
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -390,7 +390,12 @@ endef
 
 ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y)
 
+ifeq ($(BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE),y)
+UBOOT_DEPENDENCIES += zynqmp-firmware
+UBOOT_ZYNQMP_PMUFW = $(BINARIES_DIR)/pmufw.elf
+else
 UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW))
+endif
 
 ifneq ($(findstring ://,$(UBOOT_ZYNQMP_PMUFW)),)
 UBOOT_EXTRA_DOWNLOADS += $(UBOOT_ZYNQMP_PMUFW)
-- 
2.17.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-26 14:09 [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Neal Frager via buildroot
  2023-03-26 14:09 ` [Buildroot] [PATCH v1 2/2] boot/uboot.mk: new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE Neal Frager via buildroot
@ 2023-03-26 20:05 ` Thomas Petazzoni via buildroot
  2023-03-27  5:45   ` Frager, Neal via buildroot
  2023-03-27 12:21   ` Frager, Neal via buildroot
  1 sibling, 2 replies; 14+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-03-26 20:05 UTC (permalink / raw)
  To: Neal Frager via buildroot
  Cc: michal.simek, ibai.erkiaga-elorza, luca.ceresoli, Neal Frager

Hello Neal,

On Sun, 26 Mar 2023 15:09:48 +0100
Neal Frager via buildroot <buildroot@buildroot.org> wrote:

> This patch adds a new package to buildroot for building the zynqmp pmufw
> with the requirement that the user must provide an external microblaze
> toolchain.

This is not really nice :-/ Was this the conclusion of the discussion
we had on this topic?

> +config BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26
> +	bool "kria-k26"
> +	help
> +	  Adds additional CFLAGS for Kria K26 SOMs.

This doesn't look super extensible. Why are these CFLAGS needed in
particular for this platform? Is this a per-SoC or per-board
configuration? Do we expect more platforms to need custom CFLAGS?

> +ZYNQMP_FIRMWARE_VERSION = $(call qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION))
> +ZYNQMP_FIRMWARE_SOURCE = xilinx_$(ZYNQMP_FIRMWARE_VERSION).tar.gz
> +ZYNQMP_FIRMWARE_SITE = https://github.com/Xilinx/embeddedsw/archive/refs/tags

Use $(call github,...) instead

> +ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects -DBOARD_SHUTDOWN_PIN=2 -DBOARD_SHUTDOWN_PIN_STATE=0 \
> +		-DENABLE_EM -DENABLE_MOD_OVERTEMP -DENABLE_DYNAMIC_MIO_CONFIG -DENABLE_IOCTL \
> +		-DCONNECT_PMU_GPO_2_VAL=0"

All these CFLAGS are really weird... why isn't this part of the
zynqmp-firmware build system itself?

> +else
> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects"
> +endif

In any case, this should be:

ZYNQMP_CFLAGS = -Os -flto -ffat-lto-objects

ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
ZYNQMP_CFLAGS += \
	-DBOARD_SHUTDOWN_PIN=2 \
	-DBOARD_SHUTDOWN_PIN_STATE=0 \
	-DENABLE_EM \
	-DENABLE_MOD_OVERTEMP \
	-DENABLE_DYNAMIC_MIO_CONFIG \
	-DENABLE_IOCTL \
	-DCONNECT_PMU_GPO_2_VAL=0
endif

> +
> +define ZYNQMP_FIRMWARE_BUILD_CMDS
> +	cd $(@D)/lib/sw_apps/zynqmp_pmufw/src && make \
> +		COMPILER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> +		ARCHIVER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc-ar \
> +		CC=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> +		CFLAGS=$(ZYNQMP_CFLAGS)

Please use:

	$(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
		...

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 2/2] boot/uboot.mk: new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE
  2023-03-26 14:09 ` [Buildroot] [PATCH v1 2/2] boot/uboot.mk: new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE Neal Frager via buildroot
@ 2023-03-26 20:09   ` Thomas Petazzoni via buildroot
  2023-03-27 11:08     ` Frager, Neal via buildroot
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-03-26 20:09 UTC (permalink / raw)
  To: Neal Frager via buildroot
  Cc: michal.simek, ibai.erkiaga-elorza, luca.ceresoli, Neal Frager

On Sun, 26 Mar 2023 15:09:49 +0100
Neal Frager via buildroot <buildroot@buildroot.org> wrote:

> The new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE option will enable u-boot to
> use the zynqmp-firmware package for building a pmufw.elf that gets included in
> the generated boot.bin.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>  boot/uboot/Config.in | 11 +++++++++++
>  boot/uboot/uboot.mk  |  5 +++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
> index f5c20f5168..116df71ea3 100644
> --- a/boot/uboot/Config.in
> +++ b/boot/uboot/Config.in
> @@ -476,6 +476,17 @@ config BR2_TARGET_UBOOT_ZYNQMP
>  
>  if BR2_TARGET_UBOOT_ZYNQMP
>  
> +config BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE
> +	bool "U-Boot needs zynqmp-firmware"

I think this naming is confusing. If I understand correctly, U-Boot now
offers two possibilities:

(1) Using a prebuild PMU firmware (this is what is currently possible
    in Buildroot)

(2) Building the PMU firmware from source (which is what your patch is
    adding)

I believe the proposed option naming/organization doesn't make this
very clear. I don't have a super clear idea of what it should look
like. Perhaps we should have a choice...endchoice that makes it 100%
clear that the user can chose between using a pre-built vs. building
one from source?

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-26 20:05 ` [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Thomas Petazzoni via buildroot
@ 2023-03-27  5:45   ` Frager, Neal via buildroot
  2023-03-27  7:42     ` Thomas Petazzoni via buildroot
  2023-03-27 13:11     ` Luca Ceresoli via buildroot
  2023-03-27 12:21   ` Frager, Neal via buildroot
  1 sibling, 2 replies; 14+ messages in thread
From: Frager, Neal via buildroot @ 2023-03-27  5:45 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Simek, Michal, Erkiaga Elorza, Ibai, luca.ceresoli,
	Neal Frager via buildroot

Hi Thomas,

> Le 26 mars 2023 à 22:05, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit :
> 
> Hello Neal,
> 
>> On Sun, 26 Mar 2023 15:09:48 +0100
>> Neal Frager via buildroot <buildroot@buildroot.org> wrote:
>> 
>> This patch adds a new package to buildroot for building the zynqmp pmufw
>> with the requirement that the user must provide an external microblaze
>> toolchain.
> 
> This is not really nice :-/ Was this the conclusion of the discussion
> we had on this topic?

I would not say this is the conclusion of our discussion, but rather a next step.  Once we have a microblaze compiler package, we can update the zynqmp-firmware package to use the included compiler by default or allow users to select an external one, in case they prefer to use the Xilinx distributed compiler.

> 
>> +config BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26
>> +    bool "kria-k26"
>> +    help
>> +      Adds additional CFLAGS for Kria K26 SOMs.
> 
> This doesn't look super extensible. Why are these CFLAGS needed in
> particular for this platform? Is this a per-SoC or per-board
> configuration? Do we expect more platforms to need custom CFLAGS?

These CFLAGS are needed for the k26 SOMs, so we will need to include them for the kv260 and kr260 defconfigs.  I agree with you that custom flags are not so nice, and hopefully Xilinx will find a better solution for this in future versions.

For now, I think we have to live with it.

> 
>> +ZYNQMP_FIRMWARE_VERSION = $(call qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION))
>> +ZYNQMP_FIRMWARE_SOURCE = xilinx_$(ZYNQMP_FIRMWARE_VERSION).tar.gz
>> +ZYNQMP_FIRMWARE_SITE = https://github.com/Xilinx/embeddedsw/archive/refs/tags
> 
> Use $(call github,...) instead
> 

Ok

>> +ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
>> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects -DBOARD_SHUTDOWN_PIN=2 -DBOARD_SHUTDOWN_PIN_STATE=0 \
>> +        -DENABLE_EM -DENABLE_MOD_OVERTEMP -DENABLE_DYNAMIC_MIO_CONFIG -DENABLE_IOCTL \
>> +        -DCONNECT_PMU_GPO_2_VAL=0"
> 
> All these CFLAGS are really weird... why isn't this part of the
> zynqmp-firmware build system itself?
> 
>> +else
>> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects"
>> +endif
> 
> In any case, this should be:
> 
> ZYNQMP_CFLAGS = -Os -flto -ffat-lto-objects
> 
> ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> ZYNQMP_CFLAGS += \
>    -DBOARD_SHUTDOWN_PIN=2 \
>    -DBOARD_SHUTDOWN_PIN_STATE=0 \
>    -DENABLE_EM \
>    -DENABLE_MOD_OVERTEMP \
>    -DENABLE_DYNAMIC_MIO_CONFIG \
>    -DENABLE_IOCTL \
>    -DCONNECT_PMU_GPO_2_VAL=0
> endif
> 

Ok

>> +
>> +define ZYNQMP_FIRMWARE_BUILD_CMDS
>> +    cd $(@D)/lib/sw_apps/zynqmp_pmufw/src && make \
>> +        COMPILER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
>> +        ARCHIVER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc-ar \
>> +        CC=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
>> +        CFLAGS=$(ZYNQMP_CFLAGS)
> 
> Please use:
> 
>    $(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
>        ...
> 

Ok

> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com

Thanks for your review.  I will update all of this with v2 of the patch set.

Best regards,
Neal Frager
AMD

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-27  5:45   ` Frager, Neal via buildroot
@ 2023-03-27  7:42     ` Thomas Petazzoni via buildroot
  2023-03-27  9:00       ` Frager, Neal via buildroot
  2023-03-27 13:11     ` Luca Ceresoli via buildroot
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-03-27  7:42 UTC (permalink / raw)
  To: Frager, Neal
  Cc: Simek, Michal, Erkiaga Elorza, Ibai, luca.ceresoli,
	Neal Frager via buildroot

On Mon, 27 Mar 2023 05:45:45 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> I would not say this is the conclusion of our discussion, but rather
> a next step.  Once we have a microblaze compiler package, we can
> update the zynqmp-firmware package to use the included compiler by
> default or allow users to select an external one, in case they prefer
> to use the Xilinx distributed compiler.

Hm, OK. I don't know if we want to have both options of using a
compiler provided by Buildroot, or a compiler installed separately. No
strong feeling about this.

> These CFLAGS are needed for the k26 SOMs, so we will need to include
> them for the kv260 and kr260 defconfigs.  I agree with you that
> custom flags are not so nice, and hopefully Xilinx will find a better
> solution for this in future versions.
> 
> For now, I think we have to live with it.

Another solution would be to simply have a ZYNQMP_FIRMWARE option that
allows to pass custom CFLAGS, and let each defconfig handle this. Here
as well, no strong feeling about this.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-27  7:42     ` Thomas Petazzoni via buildroot
@ 2023-03-27  9:00       ` Frager, Neal via buildroot
  0 siblings, 0 replies; 14+ messages in thread
From: Frager, Neal via buildroot @ 2023-03-27  9:00 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Simek, Michal, Erkiaga Elorza, Ibai, luca.ceresoli,
	Neal Frager via buildroot

Hi Thomas,

> Le 27 mars 2023 à 09:42, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit :
> 
> On Mon, 27 Mar 2023 05:45:45 +0000
> "Frager, Neal" <neal.frager@amd.com> wrote:
> 
>> I would not say this is the conclusion of our discussion, but rather
>> a next step.  Once we have a microblaze compiler package, we can
>> update the zynqmp-firmware package to use the included compiler by
>> default or allow users to select an external one, in case they prefer
>> to use the Xilinx distributed compiler.
> 
> Hm, OK. I don't know if we want to have both options of using a
> compiler provided by Buildroot, or a compiler installed separately. No
> strong feeling about this.

As we are aware of zynqmp buildroot users who are using the Xilinx ARM compiler as an external toolchain, I think it would be good to offer this option.  However, I will change the naming to include “external”, so it is obvious that it will be an option for using a toolchain from outside buildroot.  When we have an internal microblaze toolchain ready for use, we will update the zynqmp-firmware package to make the internal toolchain the default option.

> 
>> These CFLAGS are needed for the k26 SOMs, so we will need to include
>> them for the kv260 and kr260 defconfigs.  I agree with you that
>> custom flags are not so nice, and hopefully Xilinx will find a better
>> solution for this in future versions.
>> 
>> For now, I think we have to live with it.
> 
> Another solution would be to simply have a ZYNQMP_FIRMWARE option that
> allows to pass custom CFLAGS, and let each defconfig handle this. Here
> as well, no strong feeling about this.

Actually, passing a custom CFLAGS option has its merits.  When users transition to their own custom hardware, they may be using different pmufw configurations such as enabling some of the security features of the application which are not enabled by default.

It is nicer for users to be able to change the configuration from their board config file rather than modifying the zynqmp-firmware package files.

I will include this change with v2.

> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 2/2] boot/uboot.mk: new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE
  2023-03-26 20:09   ` Thomas Petazzoni via buildroot
@ 2023-03-27 11:08     ` Frager, Neal via buildroot
  0 siblings, 0 replies; 14+ messages in thread
From: Frager, Neal via buildroot @ 2023-03-27 11:08 UTC (permalink / raw)
  To: Thomas Petazzoni, luca.ceresoli
  Cc: Neal Frager via buildroot, Simek, Michal, Erkiaga Elorza, Ibai

Hi Thomas, Luca,

> The new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE option will enable 
> u-boot to use the zynqmp-firmware package for building a pmufw.elf 
> that gets included in the generated boot.bin.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>  boot/uboot/Config.in | 11 +++++++++++  boot/uboot/uboot.mk  |  5 
> +++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in index 
> f5c20f5168..116df71ea3 100644
> --- a/boot/uboot/Config.in
> +++ b/boot/uboot/Config.in
> @@ -476,6 +476,17 @@ config BR2_TARGET_UBOOT_ZYNQMP
>  
>  if BR2_TARGET_UBOOT_ZYNQMP
>  
> +config BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE
> +	bool "U-Boot needs zynqmp-firmware"

> I think this naming is confusing. If I understand correctly, U-Boot now offers two possibilities:

> (1) Using a prebuild PMU firmware (this is what is currently possible
>    in Buildroot)

> (2) Building the PMU firmware from source (which is what your patch is
>    adding)

> I believe the proposed option naming/organization doesn't make this very clear. I don't have a super clear idea of what it should look like. Perhaps we should have a choice...endchoice that makes it 100% clear that the user can chose between using a pre-built vs. building one from source?

Yes, I agree the naming is not 100% clear.  Would you be ok with my changing the current BR2_TARGET_UBOOT_ZYNQMP_PMUFW name to BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT?  It would mean needing to change all 3 zynqmp defconfig files with the update as well as potentially breaking things for all users who currently use the BR2_TARGET_UBOOT_ZYNQMP_PMUFW config.

If you are ok with changing the name of the config, then I would propose the following:

BR2_TARGET_UBOOT_ZYNQMP_PMUFW - bool that enables the selection of either of the following two choices:
	BR2_TARGET_UBOOT_ZYNQMP_PMUFW_SOURCE
	BR2_TARGET_UBOOT_ZYNQMP_PMUFW_PREBUILT

Would you be ok with this change?  At least it would be more clear for the users.

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-26 20:05 ` [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Thomas Petazzoni via buildroot
  2023-03-27  5:45   ` Frager, Neal via buildroot
@ 2023-03-27 12:21   ` Frager, Neal via buildroot
       [not found]     ` <MN0PR12MB60041E824606160DF5277406A08B9@MN0PR12MB6004.namprd12.prod.outlook.com>
  1 sibling, 1 reply; 14+ messages in thread
From: Frager, Neal via buildroot @ 2023-03-27 12:21 UTC (permalink / raw)
  To: Thomas Petazzoni, Neal Frager via buildroot
  Cc: Simek, Michal, luca.ceresoli, Erkiaga Elorza, Ibai

Hi Thomas,

> This patch adds a new package to buildroot for building the zynqmp 
> pmufw with the requirement that the user must provide an external 
> microblaze toolchain.

> This is not really nice :-/ Was this the conclusion of the discussion we had on this topic?

> +config BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26
> +	bool "kria-k26"
> +	help
> +	  Adds additional CFLAGS for Kria K26 SOMs.

> This doesn't look super extensible. Why are these CFLAGS needed in particular for this platform? Is this a per-SoC or per-board configuration? Do we expect more platforms to need custom CFLAGS?

> +ZYNQMP_FIRMWARE_VERSION = $(call 
> +qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION))
> +ZYNQMP_FIRMWARE_SOURCE = xilinx_$(ZYNQMP_FIRMWARE_VERSION).tar.gz
> +ZYNQMP_FIRMWARE_SITE = 
> +https://github.com/Xilinx/embeddedsw/archive/refs/tags

> Use $(call github,...) instead

> +ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects -DBOARD_SHUTDOWN_PIN=2 -DBOARD_SHUTDOWN_PIN_STATE=0 \
> +		-DENABLE_EM -DENABLE_MOD_OVERTEMP -DENABLE_DYNAMIC_MIO_CONFIG -DENABLE_IOCTL \
> +		-DCONNECT_PMU_GPO_2_VAL=0"

> All these CFLAGS are really weird... why isn't this part of the zynqmp-firmware build system itself?

> +else
> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects"
> +endif

> In any case, this should be:

> ZYNQMP_CFLAGS = -Os -flto -ffat-lto-objects

> ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> ZYNQMP_CFLAGS += \
>	-DBOARD_SHUTDOWN_PIN=2 \
>	-DBOARD_SHUTDOWN_PIN_STATE=0 \
>	-DENABLE_EM \
>	-DENABLE_MOD_OVERTEMP \
>	-DENABLE_DYNAMIC_MIO_CONFIG \
>	-DENABLE_IOCTL \
>	-DCONNECT_PMU_GPO_2_VAL=0
> endif

> +
> +define ZYNQMP_FIRMWARE_BUILD_CMDS
> +	cd $(@D)/lib/sw_apps/zynqmp_pmufw/src && make \
> +		COMPILER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> +		ARCHIVER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc-ar \
> +		CC=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> +		CFLAGS=$(ZYNQMP_CFLAGS)

> Please use:

>	$(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
>		...

Could I possibly ask you for some assistance?  I have implemented all of the changes except this last one.

When I use:
	$(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
		COMPILER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
		ARCHIVER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc-ar \
		CC=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
		CFLAGS=$(ZYNQMP_CFLAGS)

The build fails as it does not seem to find the relative path ../misc containing necessary include files.

But if I add a change directory command before the make, everything works fine using below:
	cd $(@D)/lib/sw_apps/zynqmp_pmufw/src
	$(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
		COMPILER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
		ARCHIVER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc-ar \
		CC=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
		CFLAGS=$(ZYNQMP_CFLAGS)

Do you have any ideas why this would be?  Normally I would think the -C option would take care of this, but I am unable to get the zynqmp-firmware package to build without a manual change dir step.

Do you have a clean way to fix this?

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
       [not found]     ` <MN0PR12MB60041E824606160DF5277406A08B9@MN0PR12MB6004.namprd12.prod.outlook.com>
@ 2023-03-27 12:47       ` Frager, Neal via buildroot
  2023-03-27 12:57         ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 14+ messages in thread
From: Frager, Neal via buildroot @ 2023-03-27 12:47 UTC (permalink / raw)
  To: Erkiaga Elorza, Ibai, Thomas Petazzoni, Neal Frager via buildroot
  Cc: Simek, Michal, luca.ceresoli

Hi Ibai,


> -----Original Message-----
> From: Frager, Neal <neal.frager@amd.com>
> Sent: 27 March 2023 13:21
> To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>; Neal Frager via 
> buildroot <buildroot@buildroot.org>
> Cc: Erkiaga Elorza, Ibai <ibai.erkiaga-elorza@amd.com>; 
> luca.ceresoli@bootlin.com; Simek, Michal <michal.simek@amd.com>
> Subject: RE: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new 
> package
> 
> Hi Thomas,
> 
> > This patch adds a new package to buildroot for building the zynqmp 
> > pmufw with the requirement that the user must provide an external 
> > microblaze toolchain.
> 
> > This is not really nice :-/ Was this the conclusion of the 
> > discussion we had on
> this topic?
> 
> > +config BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26
> > +	bool "kria-k26"
> > +	help
> > +	  Adds additional CFLAGS for Kria K26 SOMs.
> 
> > This doesn't look super extensible. Why are these CFLAGS needed in 
> > particular
> for this platform? Is this a per-SoC or per-board configuration? Do we 
> expect more platforms to need custom CFLAGS?
> 
> > +ZYNQMP_FIRMWARE_VERSION = $(call
> > +qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION))
> > +ZYNQMP_FIRMWARE_SOURCE =
> xilinx_$(ZYNQMP_FIRMWARE_VERSION).tar.gz
> > +ZYNQMP_FIRMWARE_SITE =
> > +https://github.com/Xilinx/embeddedsw/archive/refs/tags
> 
> > Use $(call github,...) instead
> 
> > +ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> > +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects -DBOARD_SHUTDOWN_PIN=2
> -DBOARD_SHUTDOWN_PIN_STATE=0 \
> > +		-DENABLE_EM -DENABLE_MOD_OVERTEMP -
> DENABLE_DYNAMIC_MIO_CONFIG -DENABLE_IOCTL \
> > +		-DCONNECT_PMU_GPO_2_VAL=0"
> 
> > All these CFLAGS are really weird... why isn't this part of the 
> > zynqmp-firmware
> build system itself?
> 
> > +else
> > +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects"
> > +endif
> 
> > In any case, this should be:
> 
> > ZYNQMP_CFLAGS = -Os -flto -ffat-lto-objects
> 
> > ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> > ZYNQMP_CFLAGS += \
> >	-DBOARD_SHUTDOWN_PIN=2 \
> >	-DBOARD_SHUTDOWN_PIN_STATE=0 \
> >	-DENABLE_EM \
> >	-DENABLE_MOD_OVERTEMP \
> >	-DENABLE_DYNAMIC_MIO_CONFIG \
> >	-DENABLE_IOCTL \
> >	-DCONNECT_PMU_GPO_2_VAL=0
> > endif
> 
> > +
> > +define ZYNQMP_FIRMWARE_BUILD_CMDS
> > +	cd $(@D)/lib/sw_apps/zynqmp_pmufw/src && make \
> > +
> 	COMPILER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIR
> MWARE_TOOLCHAIN_PREFIX)gcc \
> > +
> 	ARCHIVER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIR
> MWARE_TOOLCHAIN_PREFIX)gcc-ar \
> > +
> 	CC=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWAR
> E_TOOLCHAIN_PREFIX)gcc \
> > +		CFLAGS=$(ZYNQMP_CFLAGS)
> 
> > Please use:
> 
> >	$(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
> >		...
> 
> Could I possibly ask you for some assistance?  I have implemented all 
> of the changes except this last one.
> 
> When I use:
> 	$(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
> 
> 	COMPILER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(Z
> YNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
> 
> 	ARCHIVER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(Z
> YNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc-ar \
> 
> 	CC=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP
> _FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
> 		CFLAGS=$(ZYNQMP_CFLAGS)
> 
> The build fails as it does not seem to find the relative path ../misc 
> containing necessary include files.
> 
> But if I add a change directory command before the make, everything 
> works fine using below:
> 	cd $(@D)/lib/sw_apps/zynqmp_pmufw/src
> 	$(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
> 
> 	COMPILER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(Z
> YNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
> 
> 	ARCHIVER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(Z
> YNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc-ar \
> 
> 	CC=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP
> _FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
> 		CFLAGS=$(ZYNQMP_CFLAGS)
> 
> Do you have any ideas why this would be?  Normally I would think the 
> -C option would take care of this, but I am unable to get the 
> zynqmp-firmware package to build without a manual change dir step.
> 
>That's because there is a hierarchical Makefile. I mean, the Makefile you are pointing is also calling Make and providing a relative path from the expected call directory. I guess the only way to fix this would be fixing the Makefiles in the Xilinx repository to make them more flexible.

Thank you for the tip.  I am trying to figure out what the issue in the Makefile might be, but when I build outside of buildroot using the following command, the build works fine:
	make -C lib/sw_apps/zynqmp_pmufw/src \
		COMPILER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
		ARCHIVER=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc-ar \
		CC=$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_EXTERNAL_TOOLCHAIN_PREFIX)gcc \
		CFLAGS=$(ZYNQMP_CFLAGS)

It fails just with replacing "make" with "$(MAKE)".  Do you have an idea already about what would need to be fixed?

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-27 12:47       ` Frager, Neal via buildroot
@ 2023-03-27 12:57         ` Thomas Petazzoni via buildroot
  2023-03-27 13:03           ` Frager, Neal via buildroot
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-03-27 12:57 UTC (permalink / raw)
  To: Frager, Neal
  Cc: Simek, Michal, Neal Frager via buildroot, luca.ceresoli,
	Erkiaga Elorza, Ibai

On Mon, 27 Mar 2023 12:47:47 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> It fails just with replacing "make" with "$(MAKE)".  Do you have an idea already about what would need to be fixed?

Use $(MAKE1) instead of $(MAKE) then. It smells like Xilinx Makefiles
do not handle well parallel build.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-27 12:57         ` Thomas Petazzoni via buildroot
@ 2023-03-27 13:03           ` Frager, Neal via buildroot
  0 siblings, 0 replies; 14+ messages in thread
From: Frager, Neal via buildroot @ 2023-03-27 13:03 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Simek, Michal, Neal Frager via buildroot, luca.ceresoli,
	Erkiaga Elorza, Ibai

Hi Thomas,

> It fails just with replacing "make" with "$(MAKE)".  Do you have an idea already about what would need to be fixed?

> Use $(MAKE1) instead of $(MAKE) then. It smells like Xilinx Makefiles do not handle well parallel build.

Thank you!  I confirm that $(MAKE1) fixes the problem.

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-27  5:45   ` Frager, Neal via buildroot
  2023-03-27  7:42     ` Thomas Petazzoni via buildroot
@ 2023-03-27 13:11     ` Luca Ceresoli via buildroot
  2023-03-27 13:22       ` Frager, Neal via buildroot
  1 sibling, 1 reply; 14+ messages in thread
From: Luca Ceresoli via buildroot @ 2023-03-27 13:11 UTC (permalink / raw)
  To: Frager, Neal
  Cc: Erkiaga Elorza, Ibai, Simek, Michal, Thomas Petazzoni,
	Neal Frager via buildroot

Hi Neal,

On Mon, 27 Mar 2023 05:45:45 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> Hi Thomas,
> 
> > Le 26 mars 2023 à 22:05, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit :
> > 
> > Hello Neal,
> >   
> >> On Sun, 26 Mar 2023 15:09:48 +0100
> >> Neal Frager via buildroot <buildroot@buildroot.org> wrote:
> >> 
> >> This patch adds a new package to buildroot for building the zynqmp pmufw
> >> with the requirement that the user must provide an external microblaze
> >> toolchain.  
> > 
> > This is not really nice :-/ Was this the conclusion of the discussion
> > we had on this topic?  
> 
> I would not say this is the conclusion of our discussion, but rather a next step.  Once we have a microblaze compiler package, we can update the zynqmp-firmware package to use the included compiler by default or allow users to select an external one, in case they prefer to use the Xilinx distributed compiler.
> 
> >   
> >> +config BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26
> >> +    bool "kria-k26"
> >> +    help
> >> +      Adds additional CFLAGS for Kria K26 SOMs.  
> > 
> > This doesn't look super extensible. Why are these CFLAGS needed in
> > particular for this platform? Is this a per-SoC or per-board
> > configuration? Do we expect more platforms to need custom CFLAGS?  
> 
> These CFLAGS are needed for the k26 SOMs, so we will need to include them for the kv260 and kr260 defconfigs.  I agree with you that custom flags are not so nice, and hopefully Xilinx will find a better solution for this in future versions.
> 
> For now, I think we have to live with it.
>
> >   
> >> +ZYNQMP_FIRMWARE_VERSION = $(call qstrip,$(BR2_PACKAGE_ZYNQMP_FIRMWARE_VERSION))
> >> +ZYNQMP_FIRMWARE_SOURCE = xilinx_$(ZYNQMP_FIRMWARE_VERSION).tar.gz
> >> +ZYNQMP_FIRMWARE_SITE = https://github.com/Xilinx/embeddedsw/archive/refs/tags  
> > 
> > Use $(call github,...) instead
> >   
> 
> Ok
> 
> >> +ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> >> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects -DBOARD_SHUTDOWN_PIN=2 -DBOARD_SHUTDOWN_PIN_STATE=0 \
> >> +        -DENABLE_EM -DENABLE_MOD_OVERTEMP -DENABLE_DYNAMIC_MIO_CONFIG -DENABLE_IOCTL \
> >> +        -DCONNECT_PMU_GPO_2_VAL=0"  
> > 
> > All these CFLAGS are really weird... why isn't this part of the
> > zynqmp-firmware build system itself?

Given you (Neal) are working on this, and you are already active in fixing
some issues in the embeddedsw repository (which is greatly appreciated
here!), why not trying to have a proper solution in there in the first
place?

Fixing things upstream can possibly take time, but in the end it will avoid
adding cruft&hacks in various places, of which I can count 3 now: yocto,
zynqmp-pmufw-builder, and hopefully soon Buildroot.

> >> +else
> >> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects"
> >> +endif  
> > 
> > In any case, this should be:
> > 
> > ZYNQMP_CFLAGS = -Os -flto -ffat-lto-objects
> > 
> > ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> > ZYNQMP_CFLAGS += \
> >    -DBOARD_SHUTDOWN_PIN=2 \
> >    -DBOARD_SHUTDOWN_PIN_STATE=0 \
> >    -DENABLE_EM \
> >    -DENABLE_MOD_OVERTEMP \
> >    -DENABLE_DYNAMIC_MIO_CONFIG \
> >    -DENABLE_IOCTL \
> >    -DCONNECT_PMU_GPO_2_VAL=0
> > endif
> >   
> 
> Ok
> 
> >> +
> >> +define ZYNQMP_FIRMWARE_BUILD_CMDS
> >> +    cd $(@D)/lib/sw_apps/zynqmp_pmufw/src && make \
> >> +        COMPILER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> >> +        ARCHIVER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc-ar \
> >> +        CC=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> >> +        CFLAGS=$(ZYNQMP_CFLAGS)  
> > 
> > Please use:
> > 
> >    $(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
> >        ...
> >   

Even better: should you use $(MAKE1) here, would it be possible to get
rid of the 'make -j1' 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] 14+ messages in thread
* Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package
  2023-03-27 13:11     ` Luca Ceresoli via buildroot
@ 2023-03-27 13:22       ` Frager, Neal via buildroot
  0 siblings, 0 replies; 14+ messages in thread
From: Frager, Neal via buildroot @ 2023-03-27 13:22 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: Erkiaga Elorza, Ibai, Simek, Michal, Thomas Petazzoni,
	Neal Frager via buildroot

Hi Luca,

> >> +ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> >> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects -DBOARD_SHUTDOWN_PIN=2 -DBOARD_SHUTDOWN_PIN_STATE=0 \
> >> +        -DENABLE_EM -DENABLE_MOD_OVERTEMP -DENABLE_DYNAMIC_MIO_CONFIG -DENABLE_IOCTL \
> >> +        -DCONNECT_PMU_GPO_2_VAL=0"  
> > 
> > All these CFLAGS are really weird... why isn't this part of the 
> > zynqmp-firmware build system itself?

> Given you (Neal) are working on this, and you are already active in fixing some issues in the embeddedsw repository (which is greatly appreciated here!), why not trying to have a proper solution in there in the first place?

> Fixing things upstream can possibly take time, but in the end it will avoid adding cruft&hacks in various places, of which I can count 3 now: yocto, zynqmp-pmufw-builder, and hopefully soon Buildroot.

Yes, agreed.  For now, I will go with the custom_cflags config enabling the k26 flags to come from the zynqmp_kria_xxx_defconfig files.  But in parallel, I will work internally to see if we can get a k26 build option for the pmufw which enables everything needed for the k26.  This way, we would only need to specify the SOM we are using and not have to know all of the configs required for that SOM.

> >> +else
> >> +ZYNQMP_CFLAGS = "-Os -flto -ffat-lto-objects"
> >> +endif
> > 
> > In any case, this should be:
> > 
> > ZYNQMP_CFLAGS = -Os -flto -ffat-lto-objects
> > 
> > ifeq ($(BR2_PACKAGE_ZYNQMP_FIRMWARE_KRIA_K26),y)
> > ZYNQMP_CFLAGS += \
> >    -DBOARD_SHUTDOWN_PIN=2 \
> >    -DBOARD_SHUTDOWN_PIN_STATE=0 \
> >    -DENABLE_EM \
> >    -DENABLE_MOD_OVERTEMP \
> >    -DENABLE_DYNAMIC_MIO_CONFIG \
> >    -DENABLE_IOCTL \
> >    -DCONNECT_PMU_GPO_2_VAL=0
> > endif
> >   
> 
> Ok
> 
> >> +
> >> +define ZYNQMP_FIRMWARE_BUILD_CMDS
> >> +    cd $(@D)/lib/sw_apps/zynqmp_pmufw/src && make \
> >> +        COMPILER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> >> +        ARCHIVER=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc-ar \
> >> +        CC=$(ZYNQMP_FIRMWARE_TOOLCHAIN_PATH)/$(ZYNQMP_FIRMWARE_TOOLCHAIN_PREFIX)gcc \
> >> +        CFLAGS=$(ZYNQMP_CFLAGS)
> > 
> > Please use:
> > 
> >    $(MAKE) -C $(@D)/lib/sw_apps/zynqmp_pmufw/src \
> >        ...
> >   

> Even better: should you use $(MAKE1) here, would it be possible to get rid of the 'make -j1' patch?

I just confirmed that with $(MAKE1), the 'make -j1' patch is not needed for buildroot.

Thanks everyone for your help!

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-03-27 13:22 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-26 14:09 [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Neal Frager via buildroot
2023-03-26 14:09 ` [Buildroot] [PATCH v1 2/2] boot/uboot.mk: new BR2_TARGET_UBOOT_NEEDS_ZYNQMP_FIRMWARE Neal Frager via buildroot
2023-03-26 20:09   ` Thomas Petazzoni via buildroot
2023-03-27 11:08     ` Frager, Neal via buildroot
2023-03-26 20:05 ` [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package Thomas Petazzoni via buildroot
2023-03-27  5:45   ` Frager, Neal via buildroot
2023-03-27  7:42     ` Thomas Petazzoni via buildroot
2023-03-27  9:00       ` Frager, Neal via buildroot
2023-03-27 13:11     ` Luca Ceresoli via buildroot
2023-03-27 13:22       ` Frager, Neal via buildroot
2023-03-27 12:21   ` Frager, Neal via buildroot
     [not found]     ` <MN0PR12MB60041E824606160DF5277406A08B9@MN0PR12MB6004.namprd12.prod.outlook.com>
2023-03-27 12:47       ` Frager, Neal via buildroot
2023-03-27 12:57         ` Thomas Petazzoni via buildroot
2023-03-27 13:03           ` Frager, Neal via buildroot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.