Add Xilinx ZynqMP and ZCU106 board support

The path to the binary images is very long. Since we are about to make a larger use of it, let's use a variable to make it somewhat shorter.

Signed-off-by Luca Ceresoli <[email protected]>

---
Changes v2 -> v3: none.
Changes v1 -> v2: none.

---
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

boot/arm-trusted-firmware/arm-trusted-firmware.mk +1 -0

index 4bac916e3108..212bb5049f2b 100644
@@ -25,6 +25,7 @@ endif
 ARM_TRUSTED_FIRMWARE_INSTALL_IMAGES = YES
 
 ARM_TRUSTED_FIRMWARE_PLATFORM = $(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM))
+ARM_TRUSTED_FIRMWARE_IMG_DIR = $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/release
 
 ARM_TRUSTED_FIRMWARE_MAKE_OPTS += \
 	CROSS_COMPILE="$(TARGET_CROSS)" \

Hi Luca,

Quoted 25 lines · Luca Ceresoli

The path to the binary images is very long. Since we are about to make a larger use of it, let's use a variable to make it somewhat shorter.

Signed-off-by Luca Ceresoli <[email protected]>

---
Changes v2 -> v3: none.
Changes v1 -> v2: none.

---
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

boot/arm-trusted-firmware/arm-trusted-firmware.mk +1 -0

index 4bac916e3108..212bb5049f2b 100644
@@ -25,6 +25,7 @@ endif
 ARM_TRUSTED_FIRMWARE_INSTALL_IMAGES = YES
 
 ARM_TRUSTED_FIRMWARE_PLATFORM = $(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM))
+ARM_TRUSTED_FIRMWARE_IMG_DIR = $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/release
 
 ARM_TRUSTED_FIRMWARE_MAKE_OPTS += \
 	CROSS_COMPILE="$(TARGET_CROSS)" \

boot/arm-trusted-firmware/arm-trusted-firmware.mk define ARM_TRUSTED_FIRMWARE_BUILD_CMDS · line 86

Quoted 5 lines

boot/arm-trusted-firmware/arm-trusted-firmware.mk +1 -1

@@ -82,7 +83,7 @@ define ARM_TRUSTED_FIRMWARE_BUILD_CMDS
 endef
 
 define ARM_TRUSTED_FIRMWARE_INSTALL_IMAGES_CMDS
-	cp -dpf $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/release/*.bin $(BINARIES_DIR)/
+	cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/*.bin $(BINARIES_DIR)/

Actually the release folder isn't always in that directory. For instance, the tegra platform requires another TARGET_SOC variable and the bin files are under build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/$(TARGET_SOC)/release/*.bin.
https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/plat/nvidia-tegra.rst

Don't get me wrong, your patch doesn't break anything, I just wanted to
inform you of that tegra case. My quick (and dirty) fix was to do a the
following:
find $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM) -name "*.bin" -exec cp -dpf {} $(BINARIES_DIR)/ \;

But if you want to have a generic ARM_TRUSTED_FIRMWARE_IMG_DIR I guess the solution is to have an optional ARM_TRUSTED_FIRMWARE_TARGET_SOC variable added to arm-trusted-firmware.mk.

Hi Gary,

Quoted 40 lines · Gary Bisson

Hi Luca,

Quoted 25 lines · Luca Ceresoli

The path to the binary images is very long. Since we are about to make a larger use of it, let's use a variable to make it somewhat shorter.

Signed-off-by Luca Ceresoli <[email protected]>

---
Changes v2 -> v3: none.
Changes v1 -> v2: none.

---
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

boot/arm-trusted-firmware/arm-trusted-firmware.mk +1 -0

index 4bac916e3108..212bb5049f2b 100644
@@ -25,6 +25,7 @@ endif
 ARM_TRUSTED_FIRMWARE_INSTALL_IMAGES = YES
 
 ARM_TRUSTED_FIRMWARE_PLATFORM = $(call qstrip,$(BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM))
+ARM_TRUSTED_FIRMWARE_IMG_DIR = $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/release
 
 ARM_TRUSTED_FIRMWARE_MAKE_OPTS += \
 	CROSS_COMPILE="$(TARGET_CROSS)" \

boot/arm-trusted-firmware/arm-trusted-firmware.mk define ARM_TRUSTED_FIRMWARE_BUILD_CMDS · line 86

Quoted 5 lines

boot/arm-trusted-firmware/arm-trusted-firmware.mk +1 -1

@@ -82,7 +83,7 @@ define ARM_TRUSTED_FIRMWARE_BUILD_CMDS
 endef
 
 define ARM_TRUSTED_FIRMWARE_INSTALL_IMAGES_CMDS
-	cp -dpf $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/release/*.bin $(BINARIES_DIR)/
+	cp -dpf $(ARM_TRUSTED_FIRMWARE_IMG_DIR)/*.bin $(BINARIES_DIR)/

Actually the release folder isn't always in that directory. For instance, the tegra platform requires another TARGET_SOC variable and the bin files are under build/$(ARM_TRUSTED_FIRMWARE_PLATFORM)/$(TARGET_SOC)/release/*.bin.
https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/plat/nvidia-tegra.rst

Quoted 4 lines

Don't get me wrong, your patch doesn't break anything, I just wanted to
inform you of that tegra case. My quick (and dirty) fix was to do a the
following:
find $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM) -name "*.bin" -exec cp -dpf {} $(BINARIES_DIR)/ \;

So if I understood correctly the current .mk file would not work for tegra, both with and without my patch.

Also your 'find' solution cannot work from ARM_TRUSTED_FIRMWARE_IMG_DIR as I defined it, because of the added '/release' dir.

Quoted 3 lines

But if you want to have a generic ARM_TRUSTED_FIRMWARE_IMG_DIR I guess the solution is to have an optional ARM_TRUSTED_FIRMWARE_TARGET_SOC variable added to arm-trusted-firmware.mk.

Well, using 'find' would be a simpler trick without clobbering the Config.in space. But then in patch 2 I use ARM_TRUSTED_FIRMWARE_IMG_DIR in several places, and using plain 'find' there would introduce additional problems.

Do you think there is a way to extract the TARGET_SOC value automatically, in order to avoid yet another Config.in variable in ATF? I'm afraid I'm not optimistic on this...

An alternative, under the assumption that only one *.bin file is generated, is: find that file wherever it is, and use the path to it everywhere to reference to it. Pseudocode (perhaps better done in pure make):

  ARM_TRUSTED_FIRMWARE_IMG = $(dir $(shell \
    find $(@D)/build/$(ARM_TRUSTED_FIRMWARE_PLATFORM) -name "*.bin" | \
    head -n1))

This would avoid any additional variables and give us the path of the binary. Do you think it could work?

The path to the binary images is very long. Since we are about to make a larger use of it, let's use a variable to make it somewhat shorter.

Signed-off-by Luca Ceresoli <[email protected]>

---
Changes v2 -> v3: none.
Changes v1 -> v2: none.

---
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

U-Boot SPL for the Xilinx ZynqMP SoCs needs ATF in this format to load it.

Signed-off-by Luca Ceresoli <[email protected]>

---
Changes v2 -> v3 (Thomas):
- Make the option not ZynqMP-specific
- Simplify the grep+sed expression to extract the entry point address
- Rather than hooks, just use directly BUILD_CMDS and INSTALL_IMAGES_CMDS
- Use $(INSTALL), not install

Hello,

boot/uboot/uboot.mk define UBOOT_INSTALL_IMAGES_CMDS · line 279

Quoted 3 lines · Luca Ceresoli

boot/uboot/uboot.mk +3 -0

+ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y)
+
+ifneq ($(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PSU_INIT_DIR)),)

It'd be nice to have an intermediate UBOOT_ZYNQMP_PSU_INIT_DIR variable instead of calculating its qstripped value twice.

boot/uboot/uboot.mk define UBOOT_INSTALL_IMAGES_CMDS · line 286

Quoted 7 lines

boot/uboot/uboot.mk +7 -0

+define UBOOT_ZYNQMP_COPY_PSU_INIT
+# In U-Boot's board/xilinx/zynqmp/Makefile the bundled psu_init_gpl.c
+# has precedence over ours if placed in a subdir whose name matches
+# CONFIG_DEFAULT_DEVICE_TREE. Delete them all to be sure we use ours.
+	rm -f $(@D)/board/xilinx/zynqmp/*/psu_init*.[ch]
+	cp $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PSU_INIT_DIR))/psu_init_gpl.[ch] \
+	   $(@D)/board/xilinx/zynqmp/

However, the bigger question is why do we need this in the first place ? This is in fact just patching the U-Boot source code. Why not use a patch instead ? Aren't people in general anyway going to have their own custom U-Boot Git tree, in which they will version control their psu_init files ?

Since this really looks like a specialized patching step, I'm not sure it really makes sense to add this. Unless you can convince me otherwise :-)

Hi Thomas,

On 28/05/2018 22:49, Thomas Petazzoni wrote:

Quoted 3 lines

Since this really looks like a specialized patching step, I'm not sure it really makes sense to add this. Unless you can convince me otherwise :-)

The psu_init files are special because they contain automatically-generated code to configure the SoC peripherals: enable devices, set pinmuxes etc, which is not otherwise done at runtime. Have a look at a few examples: [0] [1].

This file will change whenever the hardware *or* the configuration changes (e.g. to enable peripherals).

It is perfectly fine if one wants to use a mainline U-Boot release or a Xilinx release, unmodified, that works fine... *except* for the psu_init file. With your proposed workflow all users will be forced to use a custom git tree, just because they have a slightly different psu_init file. With my patch these users can just use a "standard" U-Boot (downloaded straight from the Internet), throw a psu_init file in their Buildroot boards/ dir and change their Buildroot config to point to it.

Even though it's technically code, you can consider the psu_init somewhat like a configuration file.

Did I convince you? :-)

[0]
http://git.denx.de/?p=u-boot.git;a=blob;f=board/xilinx/zynqmp/zynqmp-zcu106-revA/psu_init_gpl.c;h=fcd6a46ad9ffa0da06c4f0a67c2397aa4128ca29;hb=HEAD
[1]
http://git.denx.de/?p=u-boot.git;a=blob;f=board/xilinx/zynqmp/zynqmp-zcu102-revA/psu_init_gpl.c;h=5f21c4747584815ba86e50a36906b6c1fe8af1ca;hb=HEAD

Regards,

Quoted 3 lines · Luca Ceresoli

Hi Thomas,

On 28/05/2018 22:49, Thomas Petazzoni wrote:

[snip]

Quoted 8 lines

Quoted 3 lines

Since this really looks like a specialized patching step, I'm not sure it really makes sense to add this. Unless you can convince me otherwise :-)

The psu_init files are special because they contain automatically-generated code to configure the SoC peripherals: enable devices, set pinmuxes etc, which is not otherwise done at runtime. Have a look at a few examples: [0] [1].

Regards,
Arnout

Quoted 12 lines

This file will change whenever the hardware *or* the configuration changes (e.g. to enable peripherals).

It is perfectly fine if one wants to use a mainline U-Boot release or a Xilinx release, unmodified, that works fine... *except* for the psu_init file. With your proposed workflow all users will be forced to use a custom git tree, just because they have a slightly different psu_init file. With my patch these users can just use a "standard" U-Boot (downloaded straight from the Internet), throw a psu_init file in their Buildroot boards/ dir and change their Buildroot config to point to it.

Quoted 2 lines

Even though it's technically code, you can consider the psu_init somewhat like a configuration file.

Yeah, I guess you could say it's a kind of dts but as a .c file instead of a
.dts file. Right?

boot/uboot/uboot.mk +2 -0

index cbb899515181..266e42698d1b 100644
@@ -276,6 +276,21 @@ endef
 
 ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y)
 
+UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW))
+

Hello,

Quoted 10 lines · Luca Ceresoli

boot/uboot/uboot.mk +2 -0

index cbb899515181..266e42698d1b 100644
@@ -276,6 +276,21 @@ endef
 
 ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y)
 
+UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW))
+

boot/uboot/uboot.mk endef · line 290

Quoted 10 lines

boot/uboot/uboot.mk +10 -0

+ifneq ($(UBOOT_ZYNQMP_PMUFW),)
+UBOOT_EXTRA_DOWNLOADS += $(UBOOT_ZYNQMP_PMUFW)
+BR_NO_CHECK_HASH_FOR += $(notdir $(UBOOT_ZYNQMP_PMUFW))
+define UBOOT_ZYNQMP_KCONFIG_FIXUP
+	$(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"board/xilinx/zynqmp/pmufw.bin",$(@D)/.config)
+endef
+define UBOOT_ZYNQMP_COPY_PMUFW
+	$(INSTALL) -D -m 0644 $(UBOOT_DL_DIR)/$(notdir $(UBOOT_ZYNQMP_PMUFW)) \
+		$(@D)/board/xilinx/zynqmp/pmufw.bin
+endef

I'm wondering if your solution that consists in copying the PMU firmware to the U-Boot source directory and then using that location in the U-Boot configuration is better, or if we should simply make the U-Boot configuration point to the location in $(UBOOT_DL_DIR), i.e:

$(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,$(UBOOT_DL_DIR)/$(notdir $(UBOOT_ZYNQMP_PMUFW)),$(@D)/.config)

I'm not saying we _must_ do this, I'm wondering if it isn't simpler.

boot/uboot/uboot.mk UBOOT_DEPENDENCIES += host-mkpimage · line 359

Quoted 3 lines

boot/uboot/uboot.mk +3 -0

+define UBOOT_KCONFIG_FIXUP_CMDS
+	$(UBOOT_ZYNQMP_KCONFIG_FIXUP)
+endef

This obviously won't work with a non-kconfig capable U-Boot version, though admittedly ZynqMP support is sufficiently new in upstream U-Boot that we're pretty much guaranteed that someone trying to build for ZynqMP will have a kconfig-capable U-Boot.

But to be sure, perhaps the BR2_TARGET_UBOOT_ZYNQMP_PMUFW should have a "depends on BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG" ?

Hi Thomas,

Quoted 31 lines · Thomas Petazzoni

Hello,

Quoted 10 lines · Luca Ceresoli

boot/uboot/uboot.mk +2 -0

index cbb899515181..266e42698d1b 100644
@@ -276,6 +276,21 @@ endef
 
 ifeq ($(BR2_TARGET_UBOOT_ZYNQMP),y)
 
+UBOOT_ZYNQMP_PMUFW = $(call qstrip,$(BR2_TARGET_UBOOT_ZYNQMP_PMUFW))
+

boot/uboot/uboot.mk endef · line 290

Quoted 10 lines

boot/uboot/uboot.mk +10 -0

+ifneq ($(UBOOT_ZYNQMP_PMUFW),)
+UBOOT_EXTRA_DOWNLOADS += $(UBOOT_ZYNQMP_PMUFW)
+BR_NO_CHECK_HASH_FOR += $(notdir $(UBOOT_ZYNQMP_PMUFW))
+define UBOOT_ZYNQMP_KCONFIG_FIXUP
+	$(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,"board/xilinx/zynqmp/pmufw.bin",$(@D)/.config)
+endef
+define UBOOT_ZYNQMP_COPY_PMUFW
+	$(INSTALL) -D -m 0644 $(UBOOT_DL_DIR)/$(notdir $(UBOOT_ZYNQMP_PMUFW)) \
+		$(@D)/board/xilinx/zynqmp/pmufw.bin
+endef

I'm wondering if your solution that consists in copying the PMU firmware to the U-Boot source directory and then using that location in the U-Boot configuration is better, or if we should simply make the U-Boot configuration point to the location in $(UBOOT_DL_DIR), i.e:

$(call KCONFIG_SET_OPT,CONFIG_PMUFW_INIT_FILE,$(UBOOT_DL_DIR)/$(notdir $(UBOOT_ZYNQMP_PMUFW)),$(@D)/.config)

Quoted 1 line

I'm not saying we _must_ do this, I'm wondering if it isn't simpler.

This would definitely be better, but it wouldn't work because the U-Boot makefile looks for $(srctree)/$(CONFIG_BOOT_INIT_FILE), not just $(CONFIG_BOOT_INIT_FILE). So the file must be relative to $(@D)/board/xilinx/zynqmp/, i.e. $(srctree).

Of course this could be fixed in U-Boot...

configs/zynqmp_zcu106_defconfig new file +7 -0

new file mode 100644
index 000000000000..36643b2f4cdd
@@ -0,0 +1,31 @@
+BR2_aarch64=y
+BR2_GLOBAL_PATCH_DIR="board/zynqmp/patches/"
+BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_4_9=y
+BR2_ROOTFS_POST_IMAGE_SCRIPT="board/zynqmp/post-image.sh"
+BR2_LINUX_KERNEL=y
+BR2_LINUX_KERNEL_CUSTOM_GIT=y
+BR2_LINUX_KERNEL_CUSTOM_REPO_URL="git://github.com/Xilinx/linux-xlnx.git"

Hello,

Quoted 18 lines · Luca Ceresoli

configs/zynqmp_zcu106_defconfig new file +12 -0

new file mode 100644
index 000000000000..36643b2f4cdd
@@ -0,0 +1,31 @@
+BR2_aarch64=y
+BR2_GLOBAL_PATCH_DIR="board/zynqmp/patches/"
+BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_4_9=y
+BR2_ROOTFS_POST_IMAGE_SCRIPT="board/zynqmp/post-image.sh"
+BR2_LINUX_KERNEL=y
+BR2_LINUX_KERNEL_CUSTOM_GIT=y
+BR2_LINUX_KERNEL_CUSTOM_REPO_URL="git://github.com/Xilinx/linux-xlnx.git"
+BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION="xilinx-v2017.4"
+BR2_LINUX_KERNEL_DEFCONFIG="xilinx_zynqmp"
+BR2_LINUX_KERNEL_DTS_SUPPORT=y
+BR2_LINUX_KERNEL_INTREE_DTS_NAME="xilinx/zynqmp-zcu106-revA"
+BR2_TARGET_ROOTFS_EXT2=y

configs/zynqmp_zcu106_defconfig line 17

Quoted 5 lines

configs/zynqmp_zcu106_defconfig new file +5 -0

+BR2_TARGET_ROOTFS_EXT2_4=y
+# BR2_TARGET_ROOTFS_TAR is not set
+BR2_TARGET_ARM_TRUSTED_FIRMWARE=y
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="zynqmp"
+BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31_UBOOT=y

Shouldn't we specify an explicit version for ATF, just like it's done for U-Boot and Linux ?