Add support for Raspberry Pi 3 B plus #358

johnphilby · 2019-04-10T17:00:14Z

Make use of the PLATFORM variable to set the specific
target being used "rpi3-b" or "rpi3-bplus".

Signed-off-by: Philby John philby.j@hcl.com
Signed-off-by: Vv Ramya ramyavv@hcl.com

jbech-linaro · 2019-04-11T12:02:53Z

rpi3.mk

@@ -11,7 +11,8 @@ override COMPILE_S_KERNEL  := 64
 # Need to set this before including common.mk
 BUILDROOT_GETTY_PORT ?= ttyS0
 BR2_ROOTFS_POST_BUILD_SCRIPT ?= "board/raspberrypi3-64/post-build.sh"
-
+#For Raspberry Pi 3 B plus set to rpi3-bplus


Add space after #.

jbech-linaro · 2019-04-11T12:07:48Z

rpi3.mk

@@ -11,7 +11,8 @@ override COMPILE_S_KERNEL  := 64
 # Need to set this before including common.mk
 BUILDROOT_GETTY_PORT ?= ttyS0
 BR2_ROOTFS_POST_BUILD_SCRIPT ?= "board/raspberrypi3-64/post-build.sh"
-
+#For Raspberry Pi 3 B plus set to rpi3-bplus
+PLATFORM ?= rpi3-b


I think I saw a comment from you somewhere that you couldn't use the composite form? Or did I mix that up? What I'm looking for here is something like

PLATFORM=rpi-3b PLATFORM=rpi-3bplus

Platform directory in optee_os is core/arch/arm/plat-rpi3 so PLATFORM has to be rpi3 and PLATFORM_FLAVOR may be whatever we want (for instance b or bplus).
The consequence is, the "composite" form of PLATFORM is written rpi3-something.

https://github.com/OP-TEE/optee_os/blob/master/Makefile#L32-L33

Another option would be to rename core/arch/arm/plat-rpi3 to core/arch/arm/plat-rpi, but since all variants are still 3 more or less, would it be justified?

Another option would be to rename core/arch/arm/plat-rpi3 to core/arch/arm/plat-rpi, but since all variants are still 3 more or less, would it be justified?

I was thinking about the same. My initial thought was to leave the rpi3-... and start using rpi-... instead. But as long as it's only rpi3 devices we have, it might be more work than we gain by doing so. In that case, use what I think I proposed in the other github issue about this:

PLATFORM=rpi3-b PLATFORM=rpi3-bplus

jbech-linaro · 2019-04-11T12:18:19Z

rpi3.mk

@@ -31,13 +32,21 @@ RPI3_BOOT_CONFIG	?= $(RPI3_FIRMWARE_PATH)/config.txt
 RPI3_UBOOT_ENV		?= $(ROOT)/out/uboot.env
 RPI3_UBOOT_ENV_TXT	?= $(RPI3_FIRMWARE_PATH)/uboot.env.txt
 RPI3_STOCK_FW_PATH	?= $(ROOT)/firmware
-RPI3_STOCK_FW_PATH_BOOT	?= $(RPI3_STOCK_FW_PATH)/boot
+ifeq ($(PLATFORM),rpi3-b)
+RPI3_STOCK_FW_PATH_BOOT ?= $(RPI3_STOCK_FW_PATH)/boot


When first looking at this I ask myself why does this have to be on different paths (line 36 and 38, besides the PLATFORM)? I guess you didn't want to deal with the "firmware" git? In that case I believe it's better to actually create another xml/manifest file for Rpi3-b+.

jbech-linaro · 2019-04-11T12:20:51Z

rpi3.mk

+ifeq ($(PLATFORM),rpi3-b)
+LINUX_DTB               ?= $(LINUX_PATH)/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b.dtb
+else
+LINUX_DTB               ?= $(LINUX_PATH)/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b-plus.dtb


This is good, but I noticed that the same dtb is showing up on several places, which might be a bit confusing. We should try clean that up at some point in the future.

$ find . -name "bcm2710-rpi-3-b.dtb" ./build/rpi3/firmware/rpi3-bplus/bcm2710-rpi-3-b.dtb ./linux/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b.dtb ./firmware/boot/bcm2710-rpi-3-b.dtb ./out-br/target/boot/bcm2710-rpi-3-b.dtb $ find . -name "bcm2710-rpi-3-b-plus.dtb" ./build/rpi3/firmware/rpi3-bplus/bcm2710-rpi-3-b-plus.dtb ./linux/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b-plus.dtb

jbech-linaro · 2019-04-11T12:21:49Z

rpi3.mk

@@ -147,10 +156,18 @@ linux-cleaner: linux-cleaner-common
 ################################################################################
 # OP-TEE
 ################################################################################
-OPTEE_OS_COMMON_FLAGS += PLATFORM=rpi3
+ifeq ($(PLATFORM),rpi3-b)


Line 159 to 162: Please update to the platform names as I proposed further up.

jbech-linaro · 2019-04-11T12:23:59Z

rpi3.mk

@@ -179,6 +197,24 @@ update_rootfs: arm-tf linux u-boot
 	@install -v -p --mode=755 $(RPI3_STOCK_FW_PATH)/boot/start_db.elf $(BUILDROOT_TARGET_ROOT)/boot/start_db.elf
 	@install -v -p --mode=755 $(RPI3_STOCK_FW_PATH)/boot/start.elf $(BUILDROOT_TARGET_ROOT)/boot/start.elf
 	@install -v -p --mode=755 $(RPI3_STOCK_FW_PATH)/boot/start_x.elf $(BUILDROOT_TARGET_ROOT)/boot/start_x.elf
+else


This else is almost unnecessary with exception of a few files. If we put all files under a common path where only $(PLATFORM) makes them different, then there should be almost no need for if/else.

jbech-linaro · 2019-04-11T12:30:25Z

rpi3/firmware/rpi3-bplus/COPYING.linux

@@ -0,0 +1,356 @@
+


I don't want to get GPL-v2 files in this git, which even more justifies creating another manifest to just clone the firmware (as done already today), instead of copy files like this.

jbech-linaro · 2019-04-11T12:31:24Z

rpi3/firmware/rpi3-bplus/LICENCE.broadcom

@@ -0,0 +1,30 @@
+Copyright (c) 2006, Broadcom Corporation.


Same here, this belongs to the firmware.git instead.

jbech-linaro · 2019-04-11T12:32:37Z

rpi3/firmware/rpi3-bplus/config.txt

@@ -0,0 +1,3 @@
+enable_uart=1


This belongs here, only thing is to change it to my proposed platform name instead rpi-3bplus.

igoropaniuk · 2019-04-12T06:47:02Z

@johnphilby Please remove kernel8.img, armstub8.bin, uboot.env and *.dtbs, as they also are being built (Linux kernel, TF-A and U-boot accordingly)

And you also don't need another copy of rpi3 firmware binaries, as the one for RPi3-B also works for RPi3-B-Plus. You can just do what Joakim suggested, creating a separate repo manifest for rpi3b-plus (and using the same link from https://github.com/OP-TEE/manifest/blob/master/rpi3.xml#L25)

johnphilby · 2019-04-13T06:50:05Z

@jbech-linaro and @igoropaniuk thank you for your review comments. I will incorporate them and resubmit again.

johnphilby · 2019-04-18T18:47:47Z

@jbech-linaro @igoropaniuk I think I have addressed your comments. Created a new manifest for supporting RPi3 B Plus. Please review the commits from both the "manifest" and "build" repo.

OP-TEE/manifest#134

jbech-linaro · 2019-04-29T08:56:07Z

This all looks good, having that said, the only difference compared to the original rpi3.mk file are two lines pointing to the DTB file. Therefore I'd like to get your opinion about eventually sharing the same Makefile for both flavors. I.e., we could select either one (see "Suggestion one" below) depending on the platform or ... we simply just include the DTB for both platforms (see "Suggestion two" below). It's not the end of the day if there is an unused DTB laying around if it otherwise simplifies things a lot. Having that said, the latter idea can only be used of both DTB's actually are being compiled when building the kernel, which after thinking about it I believe is not the case.

The downside with both of my proposals are that the user must not forget to set the PLATFORM flag when building for RPi3 B-plus which is something that we doesn't really do for other OP-TEE full developer builds.

In any case, I'm happy with it is as it is here also, we can refactor the makefiles at a later point also to better share common parts (ti/ti-common.mk is probably the best to compare with).

Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

Suggestion one:

diff --git a/rpi3.mk b/rpi3.mk
index e172571..d7918c5 100644
--- a/rpi3.mk
+++ b/rpi3.mk
@@ -37,9 +37,17 @@ OPTEE_BIN_EXTRA1	?= $(OPTEE_PATH)/out/arm/core/tee-pager_v2.bin
 OPTEE_BIN_EXTRA2	?= $(OPTEE_PATH)/out/arm/core/tee-pageable_v2.bin
 
 LINUX_IMAGE		?= $(LINUX_PATH)/arch/arm64/boot/Image
+
+ifeq ($(PLATFORM),rpi3-bplus)
+LINUX_DTB		?= $(LINUX_PATH)/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b-plus.dtb
+$(info "Building RPi3-bplus")
+else
 LINUX_DTB		?= $(LINUX_PATH)/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b.dtb
+$(info "Building RPi3b (default)")
+endif
 MODULE_OUTPUT		?= $(ROOT)/module_output
 
+
 ################################################################################
 # Targets
 ################################################################################
@@ -163,7 +171,11 @@ buildroot: update_rootfs
 update_rootfs: arm-tf linux u-boot
 	@mkdir -p --mode=755 $(BUILDROOT_TARGET_ROOT)/boot
 	@mkdir -p --mode=755 $(BUILDROOT_TARGET_ROOT)/usr/bin
+ifeq ($(PLATFORM),rpi3-bplus)
+	@install -v -p --mode=755 $(LINUX_DTB) $(BUILDROOT_TARGET_ROOT)/boot/bcm2710-rpi-3-b-plus.dtb
+else
 	@install -v -p --mode=755 $(LINUX_DTB) $(BUILDROOT_TARGET_ROOT)/boot/bcm2710-rpi-3-b.dtb
+endif
 	@install -v -p --mode=755 $(RPI3_BOOT_CONFIG) $(BUILDROOT_TARGET_ROOT)/boot/config.txt
 	@install -v -p --mode=755 $(LINUX_IMAGE) $(BUILDROOT_TARGET_ROOT)/boot/kernel8.img
 	@install -v -p --mode=755 $(ARM_TF_BOOT) $(BUILDROOT_TARGET_ROOT)/boot/armstub8.bin

Suggestion two:

diff --git a/rpi3.mk b/rpi3.mk
index e172571..4272cb2 100644
--- a/rpi3.mk
+++ b/rpi3.mk
@@ -37,9 +37,12 @@ OPTEE_BIN_EXTRA1	?= $(OPTEE_PATH)/out/arm/core/tee-pager_v2.bin
 OPTEE_BIN_EXTRA2	?= $(OPTEE_PATH)/out/arm/core/tee-pageable_v2.bin
 
 LINUX_IMAGE		?= $(LINUX_PATH)/arch/arm64/boot/Image
-LINUX_DTB		?= $(LINUX_PATH)/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b.dtb
+
+LINUX_DTB_RPI3B		?= $(LINUX_PATH)/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b.dtb
+LINUX_DTB_RPI3BPLUS	?= $(LINUX_PATH)/arch/arm64/boot/dts/broadcom/bcm2710-rpi-3-b-plus.dtb
 MODULE_OUTPUT		?= $(ROOT)/module_output
 
+
 ################################################################################
 # Targets
 ################################################################################
@@ -163,7 +166,8 @@ buildroot: update_rootfs
 update_rootfs: arm-tf linux u-boot
 	@mkdir -p --mode=755 $(BUILDROOT_TARGET_ROOT)/boot
 	@mkdir -p --mode=755 $(BUILDROOT_TARGET_ROOT)/usr/bin
-	@install -v -p --mode=755 $(LINUX_DTB) $(BUILDROOT_TARGET_ROOT)/boot/bcm2710-rpi-3-b.dtb
+	@install -v -p --mode=755 $(LINUX_DTB_RPI3B) $(BUILDROOT_TARGET_ROOT)/boot/bcm2710-rpi-3-b.dtb
+	@install -v -p --mode=755 $(LINUX_DTB_RPI3BPLUS) $(BUILDROOT_TARGET_ROOT)/boot/bcm2710-rpi-3-b-plus.dtb
 	@install -v -p --mode=755 $(RPI3_BOOT_CONFIG) $(BUILDROOT_TARGET_ROOT)/boot/config.txt
 	@install -v -p --mode=755 $(LINUX_IMAGE) $(BUILDROOT_TARGET_ROOT)/boot/kernel8.img
 	@install -v -p --mode=755 $(ARM_TF_BOOT) $(BUILDROOT_TARGET_ROOT)/boot/armstub8.bin

igoropaniuk · 2019-05-17T10:03:17Z

@jbech-linaro my two cents:
I thinkthat just copying 2 dtbs (For RPi3 and RPi3B plus) makes the deal and is the most simple solution here that works. I'll vote for this

jbech-linaro · 2019-05-22T09:56:20Z

@jbech-linaro my two cents:
I thinkthat just copying 2 dtbs (For RPi3 and RPi3B plus) makes the deal and is the most simple solution here that works. I'll vote for this

Yeah, I guess that this is the better solution as of now, but that requires that both dtb's are there. I believe the install command will complain otherwise.

igoropaniuk · 2019-05-22T10:45:27Z

@jbech-linaro AFAIK, they are already in the same rpi3 firmware tarball (there is no need to download something else), so anyway you can just copy both after unpacking the archive.

jbech-linaro · 2019-05-22T10:46:43Z

I see, well, then it's a no brainer I guess.

johnphilby · 2019-06-12T10:09:57Z

@jbech-linaro my two cents:
I thinkthat just copying 2 dtbs (For RPi3 and RPi3B plus) makes the deal and is the most simple solution here that works. I'll vote for this

@jbech-linaro In lieu of this comment and our common consensus that it is cleaner to just copy both the dtbs, I have updated the pull request.

To sum up:
a) we will avoid creating another rpi3-bplus makefile.
b) we will also avoid creating an extra manifest file too.

Please take a look.

f4ad1e3

OP-TEE/manifest@397ab27

IKyriazis · 2019-06-13T14:21:02Z

I am able to restart the job. I am letting you know because I don't know if it is intended for random people to have that permission.

jbech-linaro · 2019-06-13T14:53:20Z

I am able to restart the job. I am letting you know because I don't know if it is intended for random people to have that permission.

@IKyriazis I'm well aware of that and as long as it isn't misused I'm going to leave it like that. But if there is someone starting to misuse it, then I'll have to remove that ability for non-trusted users. Anyhow, thanks for mentioning this!

jbech-linaro · 2019-06-24T04:50:25Z

@johnphilby , sorry for the late reply, this is looking good to me.
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

jforissier · 2019-06-26T11:37:31Z

@johnphilby, would you please squash the 3 commits into one and add @jbech-linaro's Reviewed-by tag?
Then I can merge this PR.

johnphilby · 2019-06-28T06:30:38Z

@johnphilby, would you please squash the 3 commits into one and add @jbech-linaro's Reviewed-by tag?
Then I can merge this PR.

Sure. Doing that right now.

jbech-linaro · 2019-06-28T06:46:57Z

FYI, as part of testing for 3.6.0, I'm doing a test build with this PR.

jbech-linaro · 2019-06-28T07:30:37Z

FYI, as part of testing for 3.6.0, I'm doing a test build with this PR.

With the patches applied on master.

+-----------------------------------------------------
24078 subtests of which 0 failed
95 test cases of which 0 failed
0 test cases were skipped
TEE test application done!

So, if tags haven't already been applied you can add my TB also.
Tested-by: Joakim Bech <joakim.bech@linaro.org> (rpi3b)

jforissier · 2019-06-28T07:53:57Z

@jbech-linaro why is IBART failing? Wrong cross-compiler version?

14089:  Incorrect selection of kernel headers: expected 4.18.x, got 4.19.x

johnphilby · 2019-06-28T07:56:39Z

Tested-by: Joakim Bech joakim.bech@linaro.org

I will add this as well. I am just trying out this interactive rebase thingy. Did not get it right. Trying again. Apparently, "--autosquash" isn't working as I would like it to.

Copy both RPi3 B and RPi3 B Plus dtb files to enable booting both the targets. This will avoid adding a seperate rpi3-bplus makefile. Signed-off-by: Philby John <philby.j@hcl.com> Signed-off-by: Vv Ramya <ramyavv@hcl.com> Reviewed-by: Joakim Bech <joakim.bech@linaro.org> Tested-by: Joakim Bech <joakim.bech@linaro.org> # (rpi3b)

johnphilby · 2019-06-28T08:18:53Z

Guess I got it right this time.

Please verify and merge.

johnphilby@1c0e779

and

johnphilby/manifest@6379790

jbech-linaro · 2019-06-28T08:26:51Z

@jbech-linaro why is IBART failing? Wrong cross-compiler version?
14089:  Incorrect selection of kernel headers: expected 4.18.x, got 4.19.x

It's because @johnphilby branch is a bit old, it needs a rebase so it gets my related patches onto his own branch (6e167bf and e029816).

jforissier · 2019-06-28T08:29:19Z

It's because @johnphilby branch is a bit old,

OK, then I guess all is well. Let's merge this. Thanks!

johnphilby mentioned this pull request Apr 10, 2019

Add support for RPI3 B Plus #354

Closed

jbech-linaro reviewed Apr 11, 2019

View reviewed changes

jbech-linaro mentioned this pull request Jun 4, 2019

Procedure for adding Verified Boot and Trusted Firmware-A support for Rasberry Pi 3B+ OP-TEE/optee_os#3066

Closed

johnphilby force-pushed the add-support-rpi3-bplus branch from f4ad1e3 to 7fcdb7c Compare June 28, 2019 07:31

johnphilby force-pushed the add-support-rpi3-bplus branch from 7fcdb7c to 1c0e779 Compare June 28, 2019 08:13

jforissier merged commit df45374 into OP-TEE:master Jun 28, 2019

jbech-linaro mentioned this pull request Oct 7, 2019

Raspberry Pi 4 TrustZone Support OP-TEE/optee_os#3205

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Raspberry Pi 3 B plus #358

Add support for Raspberry Pi 3 B plus #358

johnphilby commented Apr 10, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

jforissier Apr 11, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

jbech-linaro Apr 11, 2019

igoropaniuk commented Apr 12, 2019

johnphilby commented Apr 13, 2019

johnphilby commented Apr 18, 2019

jbech-linaro commented Apr 29, 2019

igoropaniuk commented May 17, 2019

jbech-linaro commented May 22, 2019

igoropaniuk commented May 22, 2019

jbech-linaro commented May 22, 2019

johnphilby commented Jun 12, 2019

IKyriazis commented Jun 13, 2019

jbech-linaro commented Jun 13, 2019

jbech-linaro commented Jun 24, 2019

jforissier commented Jun 26, 2019

johnphilby commented Jun 28, 2019

jbech-linaro commented Jun 28, 2019

jbech-linaro commented Jun 28, 2019

jforissier commented Jun 28, 2019

johnphilby commented Jun 28, 2019

johnphilby commented Jun 28, 2019

jbech-linaro commented Jun 28, 2019

jforissier commented Jun 28, 2019

Add support for Raspberry Pi 3 B plus #358

Add support for Raspberry Pi 3 B plus #358

Conversation

johnphilby commented Apr 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igoropaniuk commented Apr 12, 2019

johnphilby commented Apr 13, 2019

johnphilby commented Apr 18, 2019

jbech-linaro commented Apr 29, 2019

igoropaniuk commented May 17, 2019

jbech-linaro commented May 22, 2019

igoropaniuk commented May 22, 2019

jbech-linaro commented May 22, 2019

johnphilby commented Jun 12, 2019

IKyriazis commented Jun 13, 2019

jbech-linaro commented Jun 13, 2019

jbech-linaro commented Jun 24, 2019

jforissier commented Jun 26, 2019

johnphilby commented Jun 28, 2019

jbech-linaro commented Jun 28, 2019

jbech-linaro commented Jun 28, 2019

jforissier commented Jun 28, 2019

johnphilby commented Jun 28, 2019

johnphilby commented Jun 28, 2019

jbech-linaro commented Jun 28, 2019

jforissier commented Jun 28, 2019