Re: [Buildroot] [PATCH v1 1/2] package/zynqmp-firmware: new package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

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